Re: Bare "except" dangerous to ZODB?
On Tue, 2003-02-11 at 04:13, zope-dev-request@zope.org wrote:
Chris McDonough wrote:
Could this be done by initializing a dictionary at startup keyed on thread-id that a ConflictError exception's __init__ could stick a marker into, then checking that dictionary at commit time and disallowing the commit if the marker still existed?
Yes, but Transaction objects are already keyed on the thread ID, so I think you just have to set a "doomed" flag on the transaction. Attempts to commit doomed transactions result in either ConflictError or "DoomedTransactionError". ;-) Aborting the transaction (or beginning a new transaction) resets the flag.
Shane
The doomed flag is the way zodb4 works. Each transaction has a status flag that can be one of: active, preparing, prepared, failed, committed, aborting, and aborted. Actions are only allowable in certain states. The only thing you can do to a failed transaction is abort it. And the only thing you can do to a prepared transaction is commit or abort it. I'd like to backport that to ZODB3, but I need to finish the transaction package for ZODB4 first. If the current problem is perceived to be a significant risk, we could increase the priority of the backport. One downside is that some people do strange things with transactions, like joining them during the commit; that doesn't work in ZODB4. Jeremy
Jeremy Hylton wrote:
On Tue, 2003-02-11 at 04:13, zope-dev-request@zope.org wrote:
Chris McDonough wrote:
Could this be done by initializing a dictionary at startup keyed on thread-id that a ConflictError exception's __init__ could stick a marker into, then checking that dictionary at commit time and disallowing the commit if the marker still existed?
Yes, but Transaction objects are already keyed on the thread ID, so I think you just have to set a "doomed" flag on the transaction. Attempts to commit doomed transactions result in either ConflictError or "DoomedTransactionError". ;-) Aborting the transaction (or beginning a new transaction) resets the flag.
Shane
The doomed flag is the way zodb4 works. Each transaction has a status flag that can be one of: active, preparing, prepared, failed, committed, aborting, and aborted. Actions are only allowable in certain states. The only thing you can do to a failed transaction is abort it. And the only thing you can do to a prepared transaction is commit or abort it.
I'd like to backport that to ZODB3, but I need to finish the transaction package for ZODB4 first. If the current problem is perceived to be a significant risk, we could increase the priority of the backport. One downside is that some people do strange things with transactions, like joining them during the commit; that doesn't work in ZODB4.
Actually, I learned the hard way that joining transactions during commit already stopped working when we added ordering to transaction participants. I had to change AdaptableStorage quite a bit. Oh well, I know it was for the better. :-) I added a test to testZODB.py on a new branch (shane-conflict-handling-branch) that exercises the conflict handling bug. The test currently fails. It might be simpler to go with Toby's implementation for now: add a "veto" object to the transaction that refuses any attempt to commit. But maybe your transaction states are better. Let me know what you want to do. Shane
On Tue, 2003-02-11 at 12:10, Shane Hathaway wrote:
I added a test to testZODB.py on a new branch (shane-conflict-handling-branch) that exercises the conflict handling bug. The test currently fails. It might be simpler to go with Toby's implementation for now: add a "veto" object to the transaction that refuses any attempt to commit. But maybe your transaction states are better. Let me know what you want to do.
I'd like to do the transaction states, because it would keep the code in zodb3 and zodb4 similar. Unless there's a reason to think there are problems with the transaction state approach. I didn't look carefully at the test, but if I remember the discussion last time around, the problem is with read conflicts caught outside of 2PC. In that case, we either need to mark the connection so that it votes no when it gets to prepare() or we need to veto() method. I'd prefer the vote-no-in-prepare because it keeps the API smaller, but veto() isn't so bad; maybe it's better to stop the transaction quickly. We have a similar need when things go wrong inside the persistent machinery. I can't recall the details, but I think there are comments in the code about what happens when loading an object fails -- some state transition fails at any rate. In that case, the transaction really needs to be marked as failed and further actions prevented. Jeremy
Jeremy Hylton wrote:
On Tue, 2003-02-11 at 12:10, Shane Hathaway wrote:
I added a test to testZODB.py on a new branch (shane-conflict-handling-branch) that exercises the conflict handling bug. The test currently fails. It might be simpler to go with Toby's implementation for now: add a "veto" object to the transaction that refuses any attempt to commit. But maybe your transaction states are better. Let me know what you want to do.
I'd like to do the transaction states, because it would keep the code in zodb3 and zodb4 similar. Unless there's a reason to think there are problems with the transaction state approach.
That would be fine. We only need all tests (including the new test) to pass. :-)
I didn't look carefully at the test, but if I remember the discussion last time around, the problem is with read conflicts caught outside of 2PC. In that case, we either need to mark the connection so that it votes no when it gets to prepare() or we need to veto() method. I'd prefer the vote-no-in-prepare because it keeps the API smaller, but veto() isn't so bad; maybe it's better to stop the transaction quickly.
If we have veto(), it should probably expect a string argument that explains the reason for the veto. Then if something tries to commit, we can raise VetoedError(explanation). Otherwise, it seems like failed transactions would be opaque and hard to decipher. Having said that, now it seems like marking the connection instead is the better way to go. It would require no changes to the transaction machinery, so no backporting would be necessary. Shane
Shane Hathaway wrote:
Jeremy Hylton wrote:
On Tue, 2003-02-11 at 12:10, Shane Hathaway wrote:
I added a test to testZODB.py on a new branch (shane-conflict-handling-branch) that exercises the conflict handling bug. The test currently fails. It might be simpler to go with Toby's implementation for now: add a "veto" object to the transaction that refuses any attempt to commit. But maybe your transaction states are better. Let me know what you want to do.
I'd like to do the transaction states, because it would keep the code in zodb3 and zodb4 similar. Unless there's a reason to think there are problems with the transaction state approach.
That would be fine. We only need all tests (including the new test) to pass. :-)
I didn't look carefully at the test, but if I remember the discussion last time around, the problem is with read conflicts caught outside of 2PC. In that case, we either need to mark the connection so that it votes no when it gets to prepare() or we need to veto() method. I'd prefer the vote-no-in-prepare because it keeps the API smaller, but veto() isn't so bad; maybe it's better to stop the transaction quickly.
If we have veto(), it should probably expect a string argument that explains the reason for the veto. Then if something tries to commit, we can raise VetoedError(explanation). Otherwise, it seems like failed transactions would be opaque and hard to decipher.
hm this is starting to look an awful lot like Persistent Java Beans. maybe you should take a look there and borrow some architecture. Sloot.
Romain Slootmaekers wrote:
Shane Hathaway wrote:
If we have veto(), it should probably expect a string argument that explains the reason for the veto. Then if something tries to commit, we can raise VetoedError(explanation). Otherwise, it seems like failed transactions would be opaque and hard to decipher.
hm this is starting to look an awful lot like Persistent Java Beans. maybe you should take a look there and borrow some architecture.
No, it's not really related. JBs let you veto property changes, which is not like vetoing transactions. Besides, we decided we don't need any kind of transaction veto today. Shane
On Tuesday 11 February 2003 5:21 pm, Jeremy Hylton wrote:
On Tue, 2003-02-11 at 12:10, Shane Hathaway wrote:
I'd like to do the transaction states, because it would keep the code in zodb3 and zodb4 similar.
There are application-level reasons to mark a transaction as doomed, and I would like to keep *that* code looking similar ;-). The transaction states approach would work in that context too, right? -- Toby Dickenson http://www.geminidataloggers.com/people/tdickenson
On Wed, 2003-02-12 at 04:23, Toby Dickenson wrote:
On Tuesday 11 February 2003 5:21 pm, Jeremy Hylton wrote:
On Tue, 2003-02-11 at 12:10, Shane Hathaway wrote:
I'd like to do the transaction states, because it would keep the code in zodb3 and zodb4 similar.
There are application-level reasons to mark a transaction as doomed, and I would like to keep *that* code looking similar ;-). The transaction states approach would work in that context too, right?
Here's a late answer: If an application needs to mark a transaction as doomed, it is supposed to call get_transaction().abort(). If a transactional resource manager, like a database connection, needs to mark a transaction as doomed, it: - returns False from prepare() -- the ZODB4 spelling - raises an exception in tpc_vote() -- the ZODB3 spelling Jeremy
On Thursday 20 February 2003 5:17 pm, Jeremy Hylton wrote:
There are application-level reasons to mark a transaction as doomed, and I would like to keep *that* code looking similar ;-). The transaction states approach would work in that context too, right?
Here's a late answer:
If an application needs to mark a transaction as doomed, it is supposed to call get_transaction().abort(). If a transactional resource manager, like a database connection, needs to mark a transaction as doomed, it:
- returns False from prepare() -- the ZODB4 spelling - raises an exception in tpc_vote() -- the ZODB3 spelling
Thanks for that idea. Normally in the Zope context, transaction and request boundaries are co-aligned. This assumption runs deep in zope. I can see problems because an application calling abort wont stop the publisher calling commit at the end of the request. Having application state revert mid-request to the pre-transaction state seems like a bad idea. Commiting application changes made in the second half of the request seems bad too. (All from theory - I ve not tested this) -- Toby Dickenson http://www.geminidataloggers.com/people/tdickenson
On Thu, 2003-02-20 at 13:33, Toby Dickenson wrote:
On Thursday 20 February 2003 5:17 pm, Jeremy Hylton wrote:
There are application-level reasons to mark a transaction as doomed, and I would like to keep *that* code looking similar ;-). The transaction states approach would work in that context too, right?
Here's a late answer:
If an application needs to mark a transaction as doomed, it is supposed to call get_transaction().abort(). If a transactional resource manager, like a database connection, needs to mark a transaction as doomed, it:
- returns False from prepare() -- the ZODB4 spelling - raises an exception in tpc_vote() -- the ZODB3 spelling
Thanks for that idea.
Normally in the Zope context, transaction and request boundaries are co-aligned. This assumption runs deep in zope. I can see problems because an application calling abort wont stop the publisher calling commit at the end of the request.
Good point. One possibility is that the publisher should more explicitly manage the relationship between transaction and request. It could call get_transaction() at the start of a request to get a transaction object. Store that object explicitly and call its abort() or commit() method at the end of the request.
Having application state revert mid-request to the pre-transaction state seems like a bad idea. Commiting application changes made in the second half of the request seems bad too.
That does seem like a bad idea, and explicitly using a single transaction object would prevent it. On the other hand, maybe Zope app code that detects an error should not call abort(). Perhaps the publisher should be responsible for the transaction and the app code should information the publisher that something went wrong. Jeremy
participants (4)
-
Jeremy Hylton -
Romain Slootmaekers -
Shane Hathaway -
Toby Dickenson