Add support for loadBeforeEx #11
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
navytux
added a commit
to navytux/relstorage
that referenced
this pull request
Nov 11, 2021
loadBeforeEx is like loadBefore, but simpler, provides better information for object delete records and can be more efficiently implemented by many storages: zopefoundation/ZODB#323 On RelStorage loadBefore is currently implemented via 3 SQL queries: 1) check whether object record exists at all 2) retrieve object state 3) retrieve serial of next object revision Compared to that loadBeforeEx is implemented via only one SQL query "2" from the above - "retrieve object state". It is exactly the same query that loadBefore uses and after the patch loadBefore actually invokes loadBeforeEx for step 2. This change was outlined in zopefoundation/ZODB#318 (comment) and zopefoundation/ZODB#318 (comment) and as explained in the first link this patch is also semantically coupled with zodb#484 This patch passes tests with both ZODB5 and with ZODB5+zopefoundation/ZODB#323: - when ran with ZODB5 it verifies that loadBefore implementation does not become broken. - when ran with ZODB5+zopefoundation/ZODB#323 it verifies that loadBeforeEx implementation is correct. For tests to pass with ZODB5+zopefoundation/ZODB#323 we also need zopefoundation/zc.zlibstorage#11 because without that fix zc.zlibstorage does not decompress data on loadBeforeEx.
|
Could you please rebase on |
zc.zlibstorage wraps base storage and includes generic __getattr__ to forward all unknown attribute access to base: https://github.com/zopefoundation/zc.zlibstorage/blob/6d5a3c75/src/zc/zlibstorage/__init__.py#L52-L53 But if base implements loadBeforeEx (zopefoundation/ZODB#323) the following scenario is then possible: ZODB sees that zlibstorage provides loadBeforeEx and loads data via loadBeforeEx instead of loadBefore, but the data are loaded not decompressed, and ZODB complains about "broken pickle". -> let's fix this by explicitly wrapping loadBeforeEx if base storage provides it.
navytux
force-pushed
the
y/loadBeforeEx
branch
from
5a3d125 to
9d0736b
Compare
|
Yes, sure, rebased. |
Pull Request Test Coverage Report for Build 1465884568
💛 - Coveralls |
|
Are there any news for this PR, is it valid or should it be closed? |
|
@icemac, thanks for pinging and I appologize for the delay with replying on my side. This PR is valid but it also depends on zopefoundation/ZODB#323. It should not be merged before zopefoundation/ZODB#323 is merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
zc.zlibstorage wraps base storage and includes generic getattr
to forward all unknown attribute access to base:
https://github.com/zopefoundation/zc.zlibstorage/blob/6d5a3c75/src/zc/zlibstorage/__init__.py#L52-L53
But if base implements loadBeforeEx (zopefoundation/ZODB#323)
the following scenario is then possible:
ZODB sees that zlibstorage provides loadBeforeEx and loads data via
loadBeforeEx instead of loadBefore, but the data are loaded
not decompressed, and ZODB complains about "broken pickle".
-> let's fix this by explicitly wrapping loadBeforeEx if base storage
provides it.