RE: [ZODB-Dev] Savepoints are invalidated once they are used
Tim Peters wrote at 2005-7-15 16:06 -0400: > ... >Anyway, for reasons explained before, making a savepoint would still need to >copy the index, so I don't see that this would save anything worth saving. >It might allow skipping an index copy when a _rollback_ is done (at the cost >of clumsier coding for the user), but I don't care much about that expense. Okay :-) -- Dieter ___ 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] Savepoints are invalidated once they are used
[Dieter] >>> If we do, we can (usually) create a one with the state after the >>> "restore". [Tim] >> Sorry, I couldn't follow that sentence. [Dieter] > After I have restored to a savepoint "s", then the current state is the > one saved in "s", thus the sequence: > > s.restore() # and eliminate from stack > s = transaction.savepoint() > > essentially recreates "s" and the situation I would get would "restore" > not eliminate "s" from the stack -- modulo the fact, that "s" now > contains a new savepoint object (and other bindings to the old object no > longer work as expected). An "s.push()" would eliminate this drawback (at > the expense of an additional method). Got it -- thanks. Part of the hangup was simply that what you call "restore" there is actually spelled "rollback", and for some reason I didn't grasp that you were talking about the same thing (unsure why -- seems obvious enough today!). Anyway, for reasons explained before, making a savepoint would still need to copy the index, so I don't see that this would save anything worth saving. It might allow skipping an index copy when a _rollback_ is done (at the cost of clumsier coding for the user), but I don't care much about that expense. I do care about the expense of copying the index when a savepoint is made, but all schemes so far have that expense, regardless of whether they allow multiple rollbacks to a given savepoint (Jeremy misremembered when he wrote that "the old" scheme made a copy (solely) to support multiple rollbacks; it's also needed just to support nesting, which is the primary feature of savepoints). ___ 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] Savepoints are invalidated once they are used
Tim Peters wrote at 2005-7-14 19:20 -0400: > ... [Dieter] >> If we do, we can (usually) create a one with the state after the >> "restore". [Tim] >Sorry, I couldn't follow that sentence. After I have restored to a savepoint "s", then the current state is the one saved in "s", thus the sequence: s.restore() # and eliminate from stack s = transaction.savepoint() essentially recreates "s" and the situation I would get would "restore" not eliminate "s" from the stack -- modulo the fact, that "s" now contains a new savepoint object (and other bindings to the old object no longer work as expected). An "s.push()" would eliminate this drawback (at the expense of an additional method). -- Dieter ___ 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] Savepoints are invalidated once they are used
[Jeremy Hylton] >> IIRC, the old implementation of savepoints kept a copy of the index at >> the time the savepoint was taken so that you could rollback to it >> multiple times. I don't think there's any way to avoid such a copy. [Dieter Maurer] > Maybe, we keep the original implementation (a savepoint restore deletes > this savepoint) and update the docstring? > > It is rare that we need to keep the savepoint. All implementations (the current one in SVN, "the old" one, and "the original" one) copy the index at the time a savepoint is made, but for more reasons than Jeremy gave (I suspect his memory of the old implementation's details has faded a bit ;-)). To be usable for rollback at all (even once), a savepoint has to remember the state of the TmpStore index at the time the savepoint is made -- otherwise it couldn't restore the index to that state on a rollback. If the savepoint shares its remembered index object with TmpStore, then mutations to the latter's index effectively corrupt the savepoint's remembered index. The additional copying recently added, to support rolling back to a savepoint multiple times, occurs only when a rollback is actually performed. I don't mind that expense at all. BTW, I don't expect we can know what's rare or not with these yet. Nesting is a powerful ability subtransactions couldn't approach, so it's plausible to imagine that savepoints will get used for much more than just their RAM-freeing side effects -- and most copying here is to support nesting correctly. > If we do, we can (usually) create a one with the state after the > "restore". Sorry, I couldn't follow that sentence. I would like to do less indexing copying when a savepoint is made, but don't see an obvious and effective way to get away with that. The current implementation has become "obviously correct" instead of "not obviously wrong", and that's a good place to leave it for now in the absence of dire need. ___ 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] Savepoints are invalidated once they are used
Jeremy Hylton wrote at 2005-7-11 18:37 -0700: >IIRC, the old implementation of savepoints kept a copy of the index at >the time the savepoint was taken so that you could rollback to it >multiple times. I don't think there's any way to avoid such a copy. Maybe, we keep the original implementation (a savepoint restore deletes this savepoint) and update the docstring? It is rare that we need to keep the savepoint. If we do, we can (usually) create a one with the state after the "restore". -- Dieter ___ 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] Savepoints are invalidated once they are used
[Jeremy Hylton] > I understand. The further invariant is that the index captured when a > savepoint created is immutable. Or at least acts like it. Copying is an easy-to-code and effective way to ensure that, and I don't know of a better feasible way. Capturing a lot of savepoints (or doing a lot of commit(True)) while modifying a lot of objects makes for a lot of index copying, though, effectively injecting an O(N**2) time drag (even if rollback/abort(True) is never done). A "copy on modify" flavor of dict could do much better. Since I'm sure Google could also make good use of that, I expect to see a patch on the Python tracker from you implementing that by the end of this weekend . ___ 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] Savepoints are invalidated once they are used
On 7/12/05, Tim Peters <[EMAIL PROTECTED]> wrote:
> [Jeremy Hylton]
> > IIRC, the old implementation of savepoints kept a copy of the index at
> > the time the savepoint was taken so that you could rollback to it
> > multiple times. I don't think there's any way to avoid such a copy.
>
> Right, and the current implementation did that too. The "surprise" was that
> it wasn't enough. Sketch:
>
> 1. Modify object 0.
> 2. Make savepoint 1.
>It makes a copy of the current index, say {0: 0}, and remembers
>the TmpStore size, say 100.
> 3. Fiddle around.
> 4. Rollback to savepoint 1.
>This sets the TmpStore index to the saved {0: 0}, and truncatss
>TmpStore to size 100. So far so good -- or so it seems.
> 5. Modify object 0 again, and make savepoint 2.
>This changes the TmpStore index to {0: 100}, makes of a copy of
>{0: 100} in savepoint 2, and increases TmpStore size to 200. This
>just did something horribly wrong too, although it's subtle.
> 6. Rollback to savepoint 1 again.
>Because a copy of savepoint 1's index wasn't _also_ made in
>step #4, the index savepoint 1 is holding onto mutated to
>{0: 100} during step #5 (object sharing). This (#6) step
>leaves TmpStore with (the mutated) index {0: 100} and size 100.
> 7. Reference object 0.
>Oops. The index tells us to seek to pos 100, but TmpStore has
>been truncated to 100. We get a low-level exception from
>struct.unpack() about not enough bytes to unpack the data record
>header.
>
> You can guess that I saw that happening . Step #4 also needs to copy
> the index (from the savepoint to TmpStore) instead of sharing a reference,
> although this wasn't needed so long as a savepoint could be "used" at most
> once (then mutating the savepoint's index after a rollback had no ill
> effect, as the savepoint's index could never be referenced again).
I understand. The further invariant is that the index captured when a
savepoint created is immutable.
Jeremy
___
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] Savepoints are invalidated once they are used
[Jeremy Hylton]
> IIRC, the old implementation of savepoints kept a copy of the index at
> the time the savepoint was taken so that you could rollback to it
> multiple times. I don't think there's any way to avoid such a copy.
Right, and the current implementation did that too. The "surprise" was that
it wasn't enough. Sketch:
1. Modify object 0.
2. Make savepoint 1.
It makes a copy of the current index, say {0: 0}, and remembers
the TmpStore size, say 100.
3. Fiddle around.
4. Rollback to savepoint 1.
This sets the TmpStore index to the saved {0: 0}, and truncatss
TmpStore to size 100. So far so good -- or so it seems.
5. Modify object 0 again, and make savepoint 2.
This changes the TmpStore index to {0: 100}, makes of a copy of
{0: 100} in savepoint 2, and increases TmpStore size to 200. This
just did something horribly wrong too, although it's subtle.
6. Rollback to savepoint 1 again.
Because a copy of savepoint 1's index wasn't _also_ made in
step #4, the index savepoint 1 is holding onto mutated to
{0: 100} during step #5 (object sharing). This (#6) step
leaves TmpStore with (the mutated) index {0: 100} and size 100.
7. Reference object 0.
Oops. The index tells us to seek to pos 100, but TmpStore has
been truncated to 100. We get a low-level exception from
struct.unpack() about not enough bytes to unpack the data record
header.
You can guess that I saw that happening . Step #4 also needs to copy
the index (from the savepoint to TmpStore) instead of sharing a reference,
although this wasn't needed so long as a savepoint could be "used" at most
once (then mutating the savepoint's index after a rollback had no ill
effect, as the savepoint's index could never be referenced again).
___
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] Savepoints are invalidated once they are used
IIRC, the old implementation of savepoints kept a copy of the index at the time the savepoint was taken so that you could rollback to it multiple times. I don't think there's any way to avoid such a copy. Jeremy On 7/11/05, Tim Peters <[EMAIL PROTECTED]> wrote: > [Tim Peters] > > ... > > The good news is that, while it was hard to find, it's a one-line repair. > > Alas, that wasn't the end of it either. I think I'm at the end now, and all > the tests are passing again (including new tests to provoke new problems I > found). > > A savepoint (of the data manager Connection flavor) remembers the TmpStore > index at the time the savepoint was made. When a savepoint could be used > only once, the savepoint only needed to make sure it had a copy of the index > at the time the savepoint was _made_. But when a savepoint can be reused, > even the TmpStore.reset() line > > self.index = index > > was a source of subtle bugs (later mutations to `self.index` showed up in > `index` too, and `index` there is typically taken from a savepoint's "this > is what the world looked like when I was new" state -- mutating that was > harmless before because the rollback could never be used again, but became > disastrous when allowing re-use). > > ___ > 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 > ___ 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] Savepoints are invalidated once they are used
[Tim Peters] > ... > The good news is that, while it was hard to find, it's a one-line repair. Alas, that wasn't the end of it either. I think I'm at the end now, and all the tests are passing again (including new tests to provoke new problems I found). A savepoint (of the data manager Connection flavor) remembers the TmpStore index at the time the savepoint was made. When a savepoint could be used only once, the savepoint only needed to make sure it had a copy of the index at the time the savepoint was _made_. But when a savepoint can be reused, even the TmpStore.reset() line self.index = index was a source of subtle bugs (later mutations to `self.index` showed up in `index` too, and `index` there is typically taken from a savepoint's "this is what the world looked like when I was new" state -- mutating that was harmless before because the rollback could never be used again, but became disastrous when allowing re-use). ___ 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] Savepoints are invalidated once they are used
[Tim Peters] > Something subtler than I've been able to figure out yet is going wrong, > so I made a tim-savepoint branch. All the tests pass, but ... Br. The pickle cache invalidate method (which is coded in C, so isn't visible from pdb) clears the dictionary passed to it, and when using a savepoint more than once that can end up clearing the index of the TmpStore holding the savepoint'ed data. Then the TmpStore no longer believes it holds any objects, so after that point acts as if no savepoints had ever made. The good news is that, while it was hard to find, it's a one-line repair. ___ 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] Savepoints are invalidated once they are used
[Christian Heimes]
>>> From my point of view I can't see a reason why the ZODB forbids a
>>> second rolback to the savepoint.
[Jim Fulton]
>> I agree. This should be changed.
[Tim Peters]
> Sounds good to me -- it looks easy, so I'll do it .
Something subtler than I've been able to figure out yet is going wrong, so I
made a tim-savepoint branch. All the tests pass, but ...
After my first round of changes, subtxn abort (transaction.abort(1)) tests
failed. Turned out the code implementing abort(1) relied on that
savepoint.rollback() marked `savepoint` as invalid, and that was easily
repaired (changed abort(1) to explicitly mark the subtxn savepoint as
invalid). But after thinking about it, I couldn't understand _why_ that
repair was needed (except perhaps as an optimization), and that spawned this
failing code, which doesn't use subtxns:
"""
import ZODB
from ZODB.FileStorage import FileStorage
import transaction
from BTrees.IIBTree import IIBTree
st = FileStorage("blah.fs")
db = ZODB.DB(st)
cn = db.open()
rt = cn.root()
tree = rt['tree'] = IIBTree()
tree[1] = 1
transaction.commit()
tree[1] = 2
sp2 = transaction.savepoint()
tree[1] = 3
sp3 = transaction.savepoint()
tree[1] = 4
assert tree[1] == 4
sp3.rollback()
assert tree[1] == 3
if 1: # turns out this is irrelevant; "if 0" fails the same way
tree[1] = 4
assert tree[1] == 4
sp3.rollback()
assert tree[1] == 3, (tree[1], 3)
"""
The second time sp3.rollback() is done, it seems to revert the state all the
way back to the first commit: tree[1] is back to 1 again. The same kind of
thing happened under abort(1) if the subtxn savepoint wasn't explicitly
marked as invalid. Stepping thru it under pdb didn't reveal the cause,
which means I must have stepped over the important parts ... I'll try again
later.
___
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] Savepoints are invalidated once they are used
[Christian Heimes] ... >> From my point of view I can't see a reason why the ZODB forbids a >> second rolback to the savepoint. [Jim Fulton] > I agree. This should be changed. Sounds good to me -- it looks easy, so I'll do it . ___ 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] Savepoints are invalidated once they are used
Tim Peters wrote: [Christian Heimes] ... Something else strikes me. Why am I unable to roll back to the same savepoint multiple times? Because that's how it works . Maybe Jim can explain why quickly -- offhand I'm not sure. I don't think it could be guessed from the interface docs: class InvalidSavepointRollbackError(Exception): """Attempt to rollback an invalid savepoint. A savepoint may be invalid because: - The surrounding transaction has committed or aborted. - An earlier savepoint in the same transaction has been rolled back. """ The last line there reads like it's OK to roll back to the _same_ savepoint multiple times (it's not earlier than itself ...). Yup > But transaction/savepoint.txt explicitly says (and tests that) you can't, so it's intended behavior: Yup, but I think it is incorrect behavior. Jim -- Jim Fulton mailto:[EMAIL PROTECTED] Python Powered! CTO (540) 361-1714http://www.python.org Zope Corporation http://www.zope.com http://www.zope.org ___ 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] Savepoints are invalidated once they are used
[Tim Peters] ... > OTOH, the transaction.Savepoint.rollback() implementation is a bit > schizophrenic: it invalidates self, but leaves it in the transaction's > stack (for savepoints "after" self, it both invalidates them and removes > them from the transaction's stack). Oops! Not true. It does remove self from the stack. ___ 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] Savepoints are invalidated once they are used
Christian Heimes wrote: Today I stumbled over an unexpected behavior of savepoints. As far as I'm able to understand savepoints they mark a well defined state in the middle of a transaction. A savepoint is invalid if its transaction is committed or another savepoint is created. Well nesting savepoints would be a nice feature but I can live w/o it. Something else strikes me. Why am I unable to roll back to the same savepoint multiple times? Pseudo code example >>> sp = transaction.savepoint() >>> dosomething() >>> sp.valid True >>> sp.rollback() >>> domore() >>> sp.valid False >>> sp.rollback() FunkyRollbackException From my point of view I can't see a reason why the ZODB forbids a second rolback to the savepoint. I agree. This should be changed. Jim -- Jim Fulton mailto:[EMAIL PROTECTED] Python Powered! CTO (540) 361-1714http://www.python.org Zope Corporation http://www.zope.com http://www.zope.org ___ 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] Savepoints are invalidated once they are used
[Christian Heimes] > Today I stumbled over an unexpected behavior of savepoints. As far as I'm > able to understand savepoints they mark a well defined state in the > middle of a transaction. Right. > A savepoint is invalid if its transaction is committed Right -- or aborted. > or another savepoint is created. No, that's not the intent. Savepoints are intended to act like a stack. Each time you make a savepoint, it (in effect) pushes the current state on the stack. You can roll back to any state on the stack, and doing so makes all the savepoints "at and after" the one you rolled back to unusable, but leaves the ones "before" it still usable. Like so: tree[1] = 1 transaction.commit() tree[1] = 2 sp2 = transaction.savepoint() # stack has sp2 tree[1] = 3 sp3 = transaction.savepoint() # stack has sp3 on top, then sp2 tree[1] = 4 assert tree[1] == 4 sp3.rollback() # stack reduced to just sp2 assert tree[1] == 3 tree[1] = 5 assert tree[1] == 5 sp2.rollback() # can still use sp2; stack becomes empty then assert tree[1] == 2 > Well nesting savepoints would be a nice feature but I can live w/o it. Nesting is very much an intended use case. If you don't think it works, show some code (maybe there's a bug not provoked by the pattern above). > Something else strikes me. Why am I unable to roll back to the same > savepoint multiple times? Because that's how it works . Maybe Jim can explain why quickly -- offhand I'm not sure. I don't think it could be guessed from the interface docs: class InvalidSavepointRollbackError(Exception): """Attempt to rollback an invalid savepoint. A savepoint may be invalid because: - The surrounding transaction has committed or aborted. - An earlier savepoint in the same transaction has been rolled back. """ The last line there reads like it's OK to roll back to the _same_ savepoint multiple times (it's not earlier than itself ...). But transaction/savepoint.txt explicitly says (and tests that) you can't, so it's intended behavior: Once a savepoint has been used, it can't be used again: >>> savepoint.rollback() >>> dm['bob-balance'] 0.0 >>> savepoint.rollback() Traceback (most recent call last): ... InvalidSavepointRollbackError OTOH, the transaction.Savepoint.rollback() implementation is a bit schizophrenic: it invalidates self, but leaves it in the transaction's stack (for savepoints "after" self, it both invalidates them and removes them from the transaction's stack). > Pseudo code example > > >>> sp = transaction.savepoint() > >>> dosomething() > >>> sp.valid > True > >>> sp.rollback() > >>> domore() > >>> sp.valid > False > >>> sp.rollback() > FunkyRollbackException > > From my point of view I can't see a reason why the ZODB forbids a > second rolback to the savepoint. ___ 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] Savepoints are invalidated once they are used
Today I stumbled over an unexpected behavior of savepoints. As far as I'm able to understand savepoints they mark a well defined state in the middle of a transaction. A savepoint is invalid if its transaction is committed or another savepoint is created. Well nesting savepoints would be a nice feature but I can live w/o it. Something else strikes me. Why am I unable to roll back to the same savepoint multiple times? Pseudo code example >>> sp = transaction.savepoint() >>> dosomething() >>> sp.valid True >>> sp.rollback() >>> domore() >>> sp.valid False >>> sp.rollback() FunkyRollbackException From my point of view I can't see a reason why the ZODB forbids a second rolback to the savepoint. Christian ___ 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
