[ZODB-Dev] FileStorage.pack: why do we need the commit_lock (so long)

[Dieter Maurer]

Hi Jeremy,

Jeremy Hylton wrote at 2003-6-12 18:02 -0400:
 > ...

Thank your for the precise and extensive information.

 > ....
 > Another alternative, I think, would be to acquire the commit lock only
 > when pack is finishing.  Once the lock is acquired, we'd need to check
 > and make sure there isn't any more data to copy.  This solution would
 > probably be better.

That's the way I do it.

However, I (also) acquire the main storage lock, because
the file descriptor is closed and reopened in the final steps
of packing. If we had:

   working thread		packing thread

     self._file.XXX()
				self._file= ...
     self._file.YYY()

then the working threads file descriptor could change during
one of its operations.

I do not know whether "FileStorage" contains code as the above
or whether it always uses:

	   file= self._file
	   ...
	   file.XXX()
	   ...
	   file.YYY()


But, I thought, it would be safer to held the main storage lock
during the final check until the file descriptor has been changed.

 > I also agree that the code could use more comments.  Your suggestions
 > about specific comments are helpful.
 > 
 > Does this explanation make sense to you?  I'm happy to elaborate, as we
 > may discover some more issues that deserve comments.

I just found the one case, I already mentioned.


All in all, your new code is *much* improved over its predecessor:
much better to understand and much more modular (which I really
appreciate as my "HCFileStorage" extends packing. Previously
it required copying the complete "pack" method monster).


Dieter