Commit 5ae48fe1 authored by Kirill Smelkov's avatar Kirill Smelkov

Undeprecate loadBefore

Dieter Maurer notes that loadBefore cannot be deprecated yet because ZEO
essentially depends on the `end_tid` information returned by loadBefore
to update its cache:

https://github.com/zopefoundation/ZODB/pull/323#issuecomment-842021970

And to remove this dependency it would require to rework ZODB caching layer:

https://github.com/zopefoundation/ZODB/pull/323#issuecomment-845917355

So we cannot deprecate loadBefore until this rework is implemented first.

-> Remove general loadBefore deprecation, and emit loadBefore vs
loadBeforeEx warning only when actually hitting a "deletion" record,
because only that case is known to lead to data corruption.
parent c577e328
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
5.6.1 (unreleased) 5.6.1 (unreleased)
================== ==================
- Introduce a new ``loadBeforeEx`` interface and deprecate ``loadBefore``: - Introduce a new ``loadBeforeEx`` interface that complements ``loadBefore``:
``loadBeforeEx`` is simpler, provides better information for object delete ``loadBeforeEx`` is simpler, provides better information for object delete
records and can be more efficiently implemented by many storages. records and can be more efficiently implemented by many storages.
``loadBeforeEx`` is used (and required) to fix a ``DemoStorage`` data corruption ``loadBeforeEx`` is used (and required) to fix a ``DemoStorage`` data corruption
......
...@@ -710,9 +710,6 @@ class IStorage(Interface): ...@@ -710,9 +710,6 @@ class IStorage(Interface):
def loadBefore(oid, tid): def loadBefore(oid, tid):
"""Load the object data written before a transaction id """Load the object data written before a transaction id
( This method is deprecated and kept for backward-compatibility.
Please use loadBeforeEx instead. )
If there isn't data before the object before the given If there isn't data before the object before the given
transaction, then None is returned, otherwise three values are transaction, then None is returned, otherwise three values are
returned: returned:
...@@ -724,6 +721,8 @@ class IStorage(Interface): ...@@ -724,6 +721,8 @@ class IStorage(Interface):
- The transaction id of the following revision, if any, or None. - The transaction id of the following revision, if any, or None.
If the object id isn't in the storage, then POSKeyError is raised. If the object id isn't in the storage, then POSKeyError is raised.
See also: IStorageLoadBeforeEx.loadBeforeEx .
""" """
def loadSerial(oid, serial): def loadSerial(oid, serial):
......
...@@ -402,21 +402,7 @@ def loadBeforeEx(storage, oid, before): ...@@ -402,21 +402,7 @@ def loadBeforeEx(storage, oid, before):
except NotImplementedError: except NotImplementedError:
pass pass
# storage does not provide IStorageLoadBeforeEx - warn + fall back to loadBefore # storage does not provide IStorageLoadBeforeEx - fall back to loadBefore
if type(storage) not in _loadBeforeExWarned:
# there is potential race around _loadBeforeExWarned access, but due to the
# GIL this race cannot result in that set corruption, and can only lead
# to us emitting the warning twice instead of just once.
# -> do not spend CPU on lock and just ignore it.
warnings.warn(
"FIXME %s does not provide loadBeforeEx - emulating it via loadBefore, but ...\n"
"\t... 1) access will be potentially slower, and\n"
"\t... 2) not full semantic of loadBeforeEx could be provided.\n"
"\t... this can lead to data corruption.\n"
"\t... -> please see https://github.com/zopefoundation/ZODB/issues/318 for details." %
type(storage), DeprecationWarning)
_loadBeforeExWarned.add(type(storage))
try: try:
r = storage.loadBefore(oid, before) r = storage.loadBefore(oid, before)
except ZODB.POSException.POSKeyError: except ZODB.POSException.POSKeyError:
...@@ -424,7 +410,22 @@ def loadBeforeEx(storage, oid, before): ...@@ -424,7 +410,22 @@ def loadBeforeEx(storage, oid, before):
if r is None: if r is None:
# object was removed; however loadBefore does not tell when. # object was removed; however loadBefore does not tell when.
# return serial=0 - this is the "data corruption" case talked about above. # return serial=0. This can, however, lead to data corruption with e.g.
# DemoStorage (https://github.com/zopefoundation/ZODB/issues/318), so
# emit corresponding warning.
if type(storage) not in _loadBeforeExWarned:
# there is potential race around _loadBeforeExWarned access, but due to the
# GIL this race cannot result in that set corruption, and can only lead
# to us emitting the warning twice instead of just once.
# -> do not spend CPU on lock and just ignore it.
warnings.warn(
"FIXME %s does not provide loadBeforeEx - emulating it via loadBefore, but ...\n"
"\t... 1) access is be potentially slower, and\n"
"\t... 2) not full semantic of loadBeforeEx could be provided.\n"
"\t... this can lead to data corruption in the presence of delete records.\n"
"\t... -> please see https://github.com/zopefoundation/ZODB/issues/318 for details." %
type(storage), PendingDeprecationWarning)
_loadBeforeExWarned.add(type(storage))
return (None, z64) return (None, z64)
data, serial, next_serial = r data, serial, next_serial = r
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment