[Glass] Git based repositories (package-cache) problem when moving extent to another stone
Dale Henrichs via Glass
glass at lists.gemtalksystems.com
Tue Oct 27 14:48:26 PDT 2015
On 10/27/2015 01:35 PM, Mariano Martinez Peck wrote:
>
>
> On Tue, Oct 27, 2015 at 4:39 PM, Dale Henrichs
> <dale.henrichs at gemtalksystems.com
> <mailto: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
>> <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"
>
>
> 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.
Okay here's a modified patch:
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: [ :ex |
Transcript
cr;
show:
'Error for: ' , self description printString , ' during
flushCache: '
, ex description printString ].
self class flushDownloadCache.
directory := nil
Could you test this one as well? ... There is still some exposure to
permission errors for the superclass (MCFileTreeRepository), but in this
case we're not dealing with a cache but the repo itself ... which is
more serious and not really repairable by an automatic method ... there
is a `project rehome` command that let's you point a different fileTree
repo and that should be the solution for this particular case ...
I'll start the process of pushing this fix to Metacello ...
Dale
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gemtalksystems.com/mailman/private/glass/attachments/20151027/c8f3d0f3/attachment-0001.html>
More information about the Glass
mailing list