Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DemoStoage should not choose its initial oid randomly #401

Closed
d-maurer opened this issue Oct 9, 2024 · 2 comments
Closed

DemoStoage should not choose its initial oid randomly #401

d-maurer opened this issue Oct 9, 2024 · 2 comments
Labels

Comments

@d-maurer
Copy link
Contributor

d-maurer commented Oct 9, 2024

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
Copy link
Contributor Author

d-maurer commented Oct 9, 2024

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.

@d-maurer d-maurer closed this as not planned Won't fix, can't repro, duplicate, stale Oct 9, 2024
@icemac icemac added the wontfix label Oct 10, 2024
@navytux
Copy link
Contributor

navytux commented Oct 11, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants