zopefoundation / ZODB

Python object-oriented database
https://zodb-docs.readthedocs.io/
Other
683 stars 92 forks source link

`DemoStoage` should not choose its initial oid randomly #401

Closed d-maurer closed 1 month ago

d-maurer commented 1 month ago

BUG/PROBLEM REPORT / FEATURE REQUEST

Any storage must guarantee that different objects get different oids. Most storages achieve this by starting with oid 0 and generating new oids sequentially in a synchronized (uninterruptable) operation.

DemoStorage layers a MappingStorage on top of any storage, its "base storage". Usually, the base storage already has assigned some oids, and DemoStorage must not assign those oids again for new objects. It chooses its starting oid randomly (random.randint(1, 1 << 62)). While this usually gives new objects new oids, it can fail non-deterministically.

The most natural choice for a DemoStorage's initial oid would be the next oid of its base storage (and prevent the base storage to assign new oids itself). However, this may interfere with a use case for DemoStorage: support a demo on a life (independently changed) storage. I therefore suggest to choose the initial oid as the base's next_oid plus 0x100000000. This allows the base storage to create about 4 * 10 ** 9 objects before its independent oid generation may conflict with that of the DemoStorage on top -- which should be sufficient for any practical case.

d-maurer commented 1 month ago

DemoStorage does not use the (storage) typical logic to generate OIDs sequentially; instead, it uses _next_oid as a potential OID candidate and verifies in new_oid that it is currently not used. If it is, a new candidate is chosen randomly. This should be safe at least if the base storage is not modified independently.

navytux commented 1 month ago

Dieter, thanks for raising this up.

I agree with your analysis. For the reference: in ZODB/go demo storage verifies that its base stays constant and shutdowns itself if it detects base change:

https://lab.nexedi.com/kirr/neo/-/blob/ee23551d/go/zodb/storage/demo/demo.go#L91-141 https://lab.nexedi.com/kirr/neo/-/commit/4fb6bd0a

Kirill