Re: [ZODB-Dev] FileStorage iteration data records missing data_txn
Hi, On Fri, 2009-01-09 at 11:39 -0500, Jim Fulton wrote: > On Jan 9, 2009, at 7:41 AM, Christian Theune wrote: > > > Hi, > > > > in ZEORaid we switched from using `restore` to `store` for replaying > > transactions from another storage and found that the data records that > > FileStorage returns aren't filling in the `data_txn` fields correctly, > > but end up writing `None` all the time. > > What makes you think this is incorrect? > > data_txn should: > > 1. Not be in the interface. It is an optimization that depends on a > peculiarity of FileStorage of a case that only arises (today) with > undo. I'm inclined to remove this optimization. In fact, in the > presence of blobs, the optimization is dubious. This implies that the > prev_txn argument to restore should be ignored. > > 2. data_txn should only be other than None for a record that was part > of an undo. It is used to avoid copying data. Rather than copying > data, we simply record the position of the earlier record that > contains the data. My thought was based on the presumption that the DataRecord interface was described correctly (which it isn't as you point out below). > > I'm not exactly sure how to fix this, but attached is a patch that > > makes > > it work for the cases I could come up with. > > > > I'd like to add this change to the trunk (I'll write tests if this > > change gets accepted). > > > Your patch is incorrect. data_txn is not the previous record > transaction id. It is the transaction id of the transaction > containing data when the current record doesn;t contain any. It is > used as an optimization by referring to data stored in an earlier > record rather than copying data to the current record. > > The relevant interface didn't originally mention this field. The > documentation you added is incorrect. I'm going to remove the field > (along with version and tid) from the interface. Ah. That's the source of the misunderstanding, thanks for clearing that up. This sounds like I should also clean up the DataRecord in the BaseStorage because that also exposes data_txn, and storage iteration protocol on ZEO transports this attribute as well. Looks like I've got to use loadBefore() to find the transaction that an object's modification was based on. Christian -- Christian Theune · c...@gocept.com gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany http://gocept.com · tel +49 345 1229889 7 · fax +49 345 1229889 1 Zope and Plone consulting and development signature.asc Description: This is a digitally signed message part ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] FileStorage iteration data records missing data_txn
On Jan 9, 2009, at 7:41 AM, Christian Theune wrote: > Hi, > > in ZEORaid we switched from using `restore` to `store` for replaying > transactions from another storage and found that the data records that > FileStorage returns aren't filling in the `data_txn` fields correctly, > but end up writing `None` all the time. What makes you think this is incorrect? data_txn should: 1. Not be in the interface. It is an optimization that depends on a peculiarity of FileStorage of a case that only arises (today) with undo. I'm inclined to remove this optimization. In fact, in the presence of blobs, the optimization is dubious. This implies that the prev_txn argument to restore should be ignored. 2. data_txn should only be other than None for a record that was part of an undo. It is used to avoid copying data. Rather than copying data, we simply record the position of the earlier record that contains the data. > I'm not exactly sure how to fix this, but attached is a patch that > makes > it work for the cases I could come up with. > > I'd like to add this change to the trunk (I'll write tests if this > change gets accepted). Your patch is incorrect. data_txn is not the previous record transaction id. It is the transaction id of the transaction containing data when the current record doesn;t contain any. It is used as an optimization by referring to data stored in an earlier record rather than copying data to the current record. The relevant interface didn't originally mention this field. The documentation you added is incorrect. I'm going to remove the field (along with version and tid) from the interface. Jim -- Jim Fulton Zope Corporation ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
[ZODB-Dev] FileStorage iteration data records missing data_txn
Hi, in ZEORaid we switched from using `restore` to `store` for replaying transactions from another storage and found that the data records that FileStorage returns aren't filling in the `data_txn` fields correctly, but end up writing `None` all the time. I'm not exactly sure how to fix this, but attached is a patch that makes it work for the cases I could come up with. I'd like to add this change to the trunk (I'll write tests if this change gets accepted). Christian -- Christian Theune · c...@gocept.com gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany http://gocept.com · tel +49 345 1229889 7 · fax +49 345 1229889 1 Zope and Plone consulting and development Index: src/ZODB/FileStorage/FileStorage.py === --- src/ZODB/FileStorage/FileStorage.py (revision 94672) +++ src/ZODB/FileStorage/FileStorage.py (working copy) @@ -1905,6 +1905,9 @@ prev_txn = None if h.plen: data = self._file.read(h.plen) +if h.prev: +prev_h = self._read_data_header(h.prev) +prev_txn = prev_h.tid else: if h.back == 0: # If the backpointer is 0, then this transaction signature.asc Description: This is a digitally signed message part ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
