[Glass] Git based repositories (package-cache) problem when moving extent to another stone

Tue Oct 27 12:39:40 PDT 2015

On 10/27/2015 11:56 AM, Mariano Martinez Peck wrote:
>
>
> On Tue, Oct 27, 2015 at 3:22 PM, Dale Henrichs 
> <dale.henrichs at gemtalksystems.com 
> <mailto:dale.henrichs at gemtalksystems.com>> wrote:
>
>
>
>     On 10/27/2015 10:50 AM, Mariano Martinez Peck wrote:
>>     Dale,
>>
>>     While that more robust #exists does help in the sense that the
>>     user gets a much better error message, I think it's not enough. 
>>     So now I get this message:
>>
>>     ...RETRY->BaselineOfSeaside3
>>     ...RETRY->BaselineOfSeaside3
>>     gofer repository error: 'GoferRepositoryError: UserDefinedError:
>>     Disk error:  for path:
>>     /opt/gemstoneAdditions/GsDevKit_home/server/stones/gs_329/logs/github-cache/GsDevKit/Seaside31/v3.1.4.2-gs/GsDevKit-Seaside31-55f1bac/repository'...ignoring
>>     ...FAILED->BaselineOfSeaside3
>     Right, but aren't we trying to figure out why the load works when
>     you remove the offending directory
>
>
> Yes, my load works when we remove the offending directory.
>
>     but does not work when you explicitly set the cache directory to nil:
>
>       MCGitHubRepository cacheDirectory: nil
>
>
> Yes, this does not fix the problem. What I found out is that besides 
> that, I could do:
>
> MCRepositoryGroup default repositoriesDo: [:rep | rep flushCache ].
>
> That WOULD have fixed my problem. However #flushCache fails too 
> because of the same error. So we are not even able to #flushCahe with 
> current code.
Ah this is the piece that I was missing ... I had misunderstood that 
this did not work either ... and I thought you were trying to track down 
why this did not clear the cache ... but I understand now that when you 
run this code, you get a permissions related error and so you "cannot 
flush the caches"
>
>
>     ... when an extent is copied to a different stone directory there
>     are potentially a number of places that need to be fixed ...
>     basically anywhere that is going to look for things on disk ...
>     the Smalltalk code that gets executed by $GS_HOME/bin/newExtent
>     currently cleans up the tODE directory dependencies and it should
>     also reset the github-cache directory (whether or not errors are
>     going to be thrown because of permission errors)...
>
>     The $GS_HOME/sys/{default|local}/client/tode-scripts/rebuildSys
>     script is intended to clean up these directory/stone dependencies
>     for tODE and GLASS and you are expected to override with
>     application-specific clean up code ....
>
>     Soooo is the above error happening before or after you have
>     cleared MCGitHubRepository class>>cacheDirectory?
>
>
> After. It has no effect for me running such code.
>
>     I am under the impression that:
>
>       MCGitHubRepository cacheDirectory: nil
>
>     isn't working for you ... am I missing something?
>
>
> You are not missing anything. That is not working for me and I don't 
> understand why it would. I am under the impression that I would also need:
>
> MCRepositoryGroup default repositoriesDo: [:rep | rep flushCache ].
>
> No?
>
>
>
>>     But my code still fail to load. There are a couple of places that
>>     should be changed. Search for the string "*MCFileTreeFileUtils
>>     current directoryExists:*"
>>     And you will find
>>
>>     --------------------
>>       MCGitHubRepository class>>resetCacheDirectoryIfInvalid
>>      MCGitBasedNetworkRepository>>directory
>>      MCGitBasedNetworkRepository class>>resetCacheDirectoryIfInvalid
>>     MCFileTreeRepository>>flushCache
>>
>>     I think those guys should handle the error and on error, the
>>     flush or reset anyway. To avoid this (adding error handling in
>>     all those places), I suggested adding:
>>
>>     MCFileTreeFileDirectoryUtils class >> directoryExistsAndValid:
>>     aDirectory
>>       ^ aDirectory exists on: Error do: [  false ]
>>
>>     And so, the above 4 methods should actually send
>>     #directoryExistsAndValid:  rather than #directoryExists
>>
>     I'm not sure that I want to try to bullet-proof this chunk of code
>     ... I am worried about what happens if you don't have write
>     permission in the first place or the disk is full or ???? there
>     are too many possible error conditions that will either continue
>     to fail or may mask the original problem if we just handle the
>     errors ...
>
>     Besides it seems to me that the problem here is that
>     MCGitBasedNetworkRepository class>>resetCacheDirectoryIfInvalid
>     (which sets the cacheDirectory nil) won't work because if I
>     understand things correctly `MCGitHubRepository cacheDirectory:
>     nil` doesn't work ... or am I missing something here ....
>
>
> Problem is that even if I do  `MCGitHubRepository cacheDirectory: nil` 
>  the MCGitHubRepository instances still have the offending directory 
> as part of the instvar 'directory' and so the MC load fails. That's 
> why I think I need this code:
>
> MCRepositoryGroup default repositoriesDo: [:rep | rep flushCache ].
>
> Which effectively clears the 'directory' from 
> the MCGitHubRepository instances. But..guess what? I cannot flush, 
> because:
>
> *MCGitBasedNetworkRepository >> flushCache*
>   "the directory acts like a cache since we download the directory 
> from a git-based repository (github, bitbucket, etc.)"
>
> *  super flushCache.*
>   self class flushDownloadCache.
> *  directory := nil*
>
>
> that method calls super and so this is executed:
>
> *MCFileTreeRepository >> flushCache*
>   "force properties to be reread ... if the directory exists, 
> otherwise let nature
>    take it's course"
>
>   super flushCache.
>   directory
>     ifNotNil: [
> *    (MCFileTreeFileUtils current directoryExists: directory)   ->>> 
> this will throw error now*
>         ifTrue: [
>           repositoryProperties := nil.
>           self repositoryProperties ] ]
>
>
> So...to conclude, THIS piece of code does seem to workaround my problem:
>
> /MCGitHubRepository cacheDirectory: nil./
> /System commit. /
> /MCGitBasedNetworkRepository allSubInstances do: [ :rep | /
> /      " cannot send #directory: becaue it really expects a directory 
> instance. /
> /      By puttin a nil in 'directory' we are able to run #flushCache. 
> See MCFileTreeRepository >> flushCache.  for more details** "/
> /      rep instVarNamed: 'directory' put: nil./
> /      rep flushCache ]./
> /System commit. /
>

I didn't realize the bit about "would work except for error" ...

No I have a good picture and I do think that MCFileTreeRepository >> 
flushCache can and should be fixed[1] ... we should be able to 
unconditionally clear the cache in the fact of an error. Here's my 
proposed fix to MCGitBasedNetworkRepository >> flushCache from the bug 
report[1]:

flushCache
   "the directory acts like a cache since we download the directory from 
a git-based repository (github, bitbucket, etc.)"

   [super flushCache] on: Error do: [:ignored | "perhaps dump something 
to Transcript?" ].
   self class flushDownloadCache.
   directory := nil

Can you test this and let me know if it solves your problem? ... if so 
I'll push out a new version of Metacello with the fix ... I've already 
patched GsDevKit_home[2] to include the flushCache logic in rebuildSys ....

I'll be interested to hear about any other issues you might have with 
copied extents ... typically there aren't a lot of places where the 
GLASS1 code relies on disk ... tODE does and I think I've got that 
covered in rebuildSys ... obviously the github repos also rely opn disk 
caches ... but I don't think that there are any others ....

Thanks for straightening me out!

Dale

[1] https://github.com/dalehenrich/metacello-work/issues/376
[2] 
https://github.com/GsDevKit/GsDevKit_home/commit/29ec9a6d0067e409aa09604c8baa1ba36b6bf998

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gemtalksystems.com/mailman/private/glass/attachments/20151027/5c3123e2/attachment.html>