[ZODB-Dev] Savepoints are invalidated once they are used

2005-07-11 Thread 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. 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


Re: [ZODB-Dev] Savepoints are invalidated once they are used

2005-07-11 Thread Jim Fulton

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

2005-07-11 Thread Jim Fulton

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 wink.  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

2005-07-11 Thread Tim Peters
[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 wink.

___
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] Re: Savepoints are invalidated once they are used

2005-07-11 Thread Christian Heimes

Jim Fulton wrote:
 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.


Great!

HTH :)

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


RE: [ZODB-Dev] Savepoints are invalidated once they are used

2005-07-11 Thread Tim Peters
[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 wink.

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

2005-07-11 Thread Tim Peters
[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

2005-07-11 Thread Tim Peters
[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


[ZODB-Dev] Re: Savepoints are invalidated once they are used

2005-07-11 Thread Christian Heimes

Tim Peters 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.



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:


[snip]


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). 


[snip]

Nesting savepoints works according to your test and I really believe you 
that they work in real live. Honestly! :)
But there is some evil code in transaction/ that is destroying 
savepoints for my use case. Committing a subtransaction using the old 
and deprecated transaction.commit(1) syntax is invalidating all 
savepoints. The invalidation would be harmless but the zcatalog has a 
nasty feature. It is committing a subtransaction + GC cleanup after 
cataloging n object in a transaction. IIRC n=10,000 by default which 
might be reached much earlier than one can imagine.


The bad code:

class Transaction(object):

[...]

def commit(self, subtransaction=False):

if self._savepoint2index:
self._invalidate_all_savepoints()

if subtransaction:
# TODO deprecate subtransactions
self._subtransaction_savepoint = self.savepoint(1)
return

[...]

What about altering the order of the two if statements? IMO there is no 
need to invalidate all savepoints when creeating a subtransaction.


Tim? Jim?

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


Re: [ZODB-Dev] Savepoints are invalidated once they are used

2005-07-11 Thread 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.

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