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

Tue Oct 27 13:35:19 PDT 2015

On Tue, Oct 27, 2015 at 4:39 PM, Dale Henrichs <
dale.henrichs at gemtalksystems.com> wrote:

>
>
> 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> 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"
>
>
Yes. So there were 2 things. First, the  ` MCGitHubRepository
cacheDirectory: nil` as not enough as I also needed to #flushCache
instances.
Second, was that I was not even able to flush caches because of permissions
related errors.

>
>>
>> ... 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
>
>
Yes, I would dump the error into the transcript.
And yes, that code does fix my problem. Of course, my workaround was to
avoid modifying #flushCache, but if we can incorporate your fix, then even
better.
It also makes the flush code easier:

*MCGitHubRepository cacheDirectory: nil.*
*MCRepositoryGroup default repositoriesDo: [:rep | rep flushCache ].*
*System commit. *

> Can you test this and let me know if it solves your problem? ...
>

yes it does, with the execution of above 3 lines. You may want to add that
to gsDevKit_home as well (in that rebuildSys thingy?). Maybe you may want
to only flush cache only those repos which are directory based...

> 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 ....
>
>
Great.

> 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 ....
>

OK, great. Yes, I will keep you informed if I found out more.

>
> Thanks for straightening me out!
>
> Dale
>
> [1] https://github.com/dalehenrich/metacello-work/issues/376
> [2]
> https://github.com/GsDevKit/GsDevKit_home/commit/29ec9a6d0067e409aa09604c8baa1ba36b6bf998
>
>
>

-- 
Mariano
http://marianopeck.wordpress.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gemtalksystems.com/mailman/private/glass/attachments/20151027/7371b419/attachment-0001.html>