Bug while packing blob storage

[Christian Theune]

This happened in a database that I was trying to pack:

2007-08-05T12:53:57 ERROR ZEO.zrpc (19406) Error raised in delayed method
Traceback (most recent call last):
  File "/home/alive/alive/eggs/tmpzAzAxj/ZODB3-3.9.0_dev_r77011-py2.4-linux-i686.egg/ZEO/StorageServer.py", line 1246, in run
  File "/home/alive/alive/eggs/tmpzAzAxj/ZODB3-3.9.0_dev_r77011-py2.4-linux-i686.egg/ZEO/StorageServer.py", line 416, in _pack_impl
  File "/home/alive/alive/eggs/ZODB3-3.9.0_dev_r77011-py2.4-linux-i686.egg/ZODB/blob.py", line 535, in pack
    self._packUndoing(packtime, referencesf)
  File "/home/alive/alive/eggs/ZODB3-3.9.0_dev_r77011-py2.4-linux-i686.egg/ZODB/blob.py", line 478, in _packUndoing
    oid = utils.repr_to_oid(oid_repr)
  File "/home/alive/alive/eggs/tmpzAzAxj/ZODB3-3.9.0_dev_r77011-py2.4-linux-i686.egg/ZODB/utils.py", line 142, in repr_to_oid
TypeError: Odd-length string

[Christian Theune (ctheune)]

Looking at the code this is a problem that is described in the first XXX comment of the _packUndoing function in the blob storage: we're not tolerant of garbage directories/files in the base directory.

[Christian Theune (ctheune)]

Cases that will currently make packing fail:

a) any file/directory that is not an oid
    (will make repr_to_oid fail)

b) all files (will make listdir() fail)

c) For garbage directories that look like oids (8-character strings) there is a small chance that they contain files that are named ".blob" and do not contain a serial (e.g. blob-dir/aintblob/notan8charactername.blob). This will currently fail because repr_to_oid in splitBlobFilename will fail.

We probably should not care case c) as IMHO we shouldn't tempt people to put random stuff in the blob directory. The more cases we ignore that harder it will be to figure out real errors in the blob storage.

[Christian Theune (ctheune)]

We spent some time on making packing more tolerant. However, I have the feeling that this makes the code much much less readable and maintenable so we did not check it in but attached it here for exemplatory purposes.

In the future we are requiring that administrators and users do not modify the data structures in the managed blob directory.

We are now going to create a separate <blobdir>/tmp/ directory in the main blob directory that avoids that the blob storage itself creates files that confuses packing.

[Christian Theune (ctheune)]

The commit to the trunk counts as a release.

[Christian Theune (ctheune)]

Released in 3.8.0c1

[sixstring (johns-anderson)]

OK, this is just lazy programming. The fact is, in the real world, that server administrators are not intimate with ZODB guts. (And, hello, there's not even a README.txt in that directory to help them out.) So, sometimes, they do things that simplistic code might not like. The code should handle that.

If the code can't handle "garbage files", then code should include a check function that validates the filesystem before it tries to pack it.

At least, it should give more meaningful messages than "Odd length string." I wasted days trying to nail this down.

Do the due diligence! That's what's required for excellence.