Re: [Zope-dev] Re: Bare except dangerous to ZODB?
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 ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Re: Bare except dangerous to ZODB?
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 ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Re: Bare except dangerous to ZODB?
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 ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: Bare except dangerous to ZODB? was Re: Accept-Charset hearderscausing 500 internal server error.[correct but not lenient]
Paul Winkler [EMAIL PROTECTED] wrote:
$ cd /usr/src/Zope-2.6.1-src/lib/python/Products/
$ find . -name *py -exec grep -H except: {} \; | wc -l
170
well, this is all stuff that comes with Zope, hopefully
they have been vetted... but then there's all these
'fraid not. These days they tend to get fixed when someone touches
the nearby code for other reasons, but no one has yet mounted
a campaign to fix them all, despite this having been a known problem
for quite some time now.
And it's generally easier just to never use a bare except than it
is to prove to yourself that the bare except isn't going to have
unintended consequences. There are exceptions where a bare
except is needed, of course.
--RDM
___
Zope-Dev maillist - [EMAIL PROTECTED]
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: Bare except dangerous to ZODB?
On Tue, 2003-02-11 at 04:13, [EMAIL PROTECTED] 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 ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Re: Bare except dangerous to ZODB?
Jeremy Hylton wrote: On Tue, 2003-02-11 at 04:13, [EMAIL PROTECTED] 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 ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Re: Bare except dangerous to ZODB?
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 ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Re: Bare except dangerous to ZODB?
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 ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Re: Bare except dangerous to ZODB?
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. ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Re: Bare except dangerous to ZODB?
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 ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: Bare except dangerous to ZODB? was Re: [Zope-dev]Accept-Charset hearders causing 500 internal server error.[correct but notlenient]
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? On Mon, 2003-02-10 at 15:47, Shane Hathaway wrote: tal:on-error also catches all exceptions. It could be made to catch all exceptions except ConflictError, but I don't feel like that's the right solution. I think the right solution is to prevent applications from committing potentially conflicting data, even when ConflictErrors are ignored. This doesn't seem to be a showstopper for now, but as more applications use ZODB, it could become a bigger problem. ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: Bare except dangerous to ZODB? was Re: [Zope-dev] Accept-Charsethearders causing 500 internal server error.[correct but not lenient]
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 On Mon, 2003-02-10 at 15:47, Shane Hathaway wrote: tal:on-error also catches all exceptions. It could be made to catch all exceptions except ConflictError, but I don't feel like that's the right solution. I think the right solution is to prevent applications from committing potentially conflicting data, even when ConflictErrors are ignored. This doesn't seem to be a showstopper for now, but as more applications use ZODB, it could become a bigger problem. ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Re: Bare except dangerous to ZODB? was Re:[Zope-dev] Accept-Charset hearders causing 500 internal servererror.[correct but not lenient]
On Mon, 2003-02-10 at 20:46, 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? +1 And logging a zLOG.ERROR (or maybe even zLOG.PANIC?) level message to that effect IMHO, something that blocked commits of ConflictError'ed transactions should be a requirement in Zope 2.7 I believe that unqualified except:'s are the most common cause of unexplained Zope ZODB corruption. -- Ideas don't stay in some minds very long because they don't like solitary confinement. ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
