Re: [ZODB-Dev] Shared/DC/ZRDB/TM.py:_register
Vincent Pelletier wrote at 2008-5-22 11:21 +0200: > ... >BTW, the usual error hook treats conflict error exceptions differently from >others, and I guess it was done so because those can happen in TPC. No, the reason is to repeat a transaction that failed due to a "ConflictError". -- 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] Shared/DC/ZRDB/TM.py:_register
(Re-send because I forgot to CC the list) Le Monday 19 May 2008 21:13:49, vous avez écrit : > Think a bit about your wish: > > As soon as one transaction ends, a new one starts. > > What should happen with your artificial error handling transaction? > Should it be aborted? or committed?. I think it should be aborted. Partly because it's what happens currently, and because I don't think there is any need to permanently modify data in the error hook. If one wants to do so, I guess he will just write an error path in his code. > What should happen when the > commit fails -- another error handling, in another error handling > transaction? I see this as an argument against commiting the error-hook transaction. > The current behaviour is good in most cases. > If you dislike it in some special cases, abort the transaction > (you will get a new one, aborted automatically at the end > of error handling, unless you do the commit). The point is that I don't see why it would be bad in those "most cases" to systematicaly abort. I did the modification I suggested in the bug report before submiting it, and the error page still renders the error traceback properly. Could you be more specific about what's missing from error hook if original transaction is aborted ? > The better alternative would be to not prevent "join"s to > a doomed transaction. Right. Only commits should be prevented (they currently are when exception happened during TPC). I'm not sure about the case where exception was raised before TPC: should commits be prevented in this case too ? Maybe it's the coder's responsibility not to call commit in the error hook. -- Vincent Pelletier ___ 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] Shared/DC/ZRDB/TM.py:_register
Le Thursday 22 May 2008 10:40:46, vous avez écrit : > Maybe, you search the "zope-dev" archive to find the thread that convinced > us to change the former behaviour (the one you want now again) > into the current one? Adding links for possible future reference: https://bugs.launchpad.net/zope2/+bug/142446 First discusses a problem with handling erorr outside of any transaction, suggesting a fix similar to mine, and then later it was agreed that failed transaction should be aborted after error handling (https://bugs.launchpad.net/zope2/+bug/142446/comments/3). > The same can apply to "__traceback_info__" and "__traceback_supplement__" > information derived from persistent objects. > This information will then reflect the persistent state as it > has been when the transaction started and not as it was when the > exception occurred. I see. Thanks. And the problem with providing a "pointer" to such information is the same as having 2 transactions simultaneously started for the same thread, I guess. > The ZODB has a notion of "doomed transaction". > A transaction gets doomed when something happens that can > can lead to persistent inconsistencies should the > transaction be committed. > > A failing "commit" and an unsuccessful reset to a savepoint > are example "doom" reasons. I am not sure whether a "ReadConflictError", > too, dooms the transaction. BTW, the usual error hook treats conflict error exceptions differently from others, and I guess it was done so because those can happen in TPC. In my case, the problem was triggered by MultipleUndoErrors, which is not specialy handled. I have the feeling that current exception class inheritance scheme is not so good, and that it would be needed to create more abstract exceptions classes to inherit from. For example, an exception class which would instruct publisher to retry transaction and another to instruct it to give up (...and of course abort transaction) as fast as possible. I have no idea about the needed amount of work. -- Vincent Pelletier ___ 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] Shared/DC/ZRDB/TM.py:_register
Vincent Pelletier wrote at 2008-5-14 02:49 +0200: >Le Tuesday 13 May 2008 20:02:42 Dieter Maurer, vous avez écrit : >> Someone convinced us that error handling should (of course) >> see the state the error happened and not a new clean state -- in >> order to be able to report about the errorneous state > >Then, in my opinion, it should not be executed "inside" what failed, but in a >clean environment with a "pointer" (in non-technical meaning) to the failed >transaction. Quite difficult and complex. >> Another reason was also: should your error template need to run >> in a fresh transaction, then just abort the old one. > >How ? IIRC it's a bad coding practice to interact with transaction mechanism >from what's considered as "inside" a transaction (ZPublisher being the >borderline). That's what you have now. The border line is the end of request processing (which includes error handling). Think a bit about your wish: As soon as one transaction ends, a new one starts. What should happen with your artificial error handling transaction? Should it be aborted? or committed?. What should happen when the commit fails -- another error handling, in another error handling transaction? The current behaviour is good in most cases. If you dislike it in some special cases, abort the transaction (you will get a new one, aborted automatically at the end of error handling, unless you do the commit). > >Maybe 2 cases should be handled differently: > - exception happened when processing transaction: do not abort immediately > - exception happened in transaction handling (hopefully only in "commit"): > abort to offer error handling a "usable" environment The better alternative would be to not prevent "join"s to a doomed transaction. -- 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] Shared/DC/ZRDB/TM.py:_register
Le Tuesday 13 May 2008 20:02:42 Dieter Maurer, vous avez écrit : > Someone convinced us that error handling should (of course) > see the state the error happened and not a new clean state -- in > order to be able to report about the errorneous state Then, in my opinion, it should not be executed "inside" what failed, but in a clean environment with a "pointer" (in non-technical meaning) to the failed transaction. Or there must be some special directives about what can or cannot happen in an error handler (but I guess it's just too hard to make sure nothing joins a transaction, for example fetching a page template from ZODB to render the error will join the transaction). > Another reason was also: should your error template need to run > in a fresh transaction, then just abort the old one. How ? IIRC it's a bad coding practice to interact with transaction mechanism from what's considered as "inside" a transaction (ZPublisher being the borderline). > If the transaction were aborted before error handling, then > an error template with different requirements does not have a chance Right. Maybe 2 cases should be handled differently: - exception happened when processing transaction: do not abort immediately - exception happened in transaction handling (hopefully only in "commit"): abort to offer error handling a "usable" environment Anyway, I opened a bug in the bugtracker about this problem, because it's a separate issue from the TM.py problem: https://bugs.launchpad.net/bugs/229863 -- Vincent Pelletier ___ 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] Shared/DC/ZRDB/TM.py:_register
Andreas Jung wrote at 2008-5-13 20:19 +0200: > ... >> "Shared.DC.ZRDB.TM.TM" is the standard Zope[2] way to implement a >> ZODB DataManager. > >Nowadays you create a datamanager implementing IDataManager and join it >with the current transaction. Shared.DC.ZRDB.TM.TM is pretty much >old-old-old-style. Time to change the Zope 2 code base ;-) There, you still find the old way -- and it is used by other Zope 2 components -- 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] Shared/DC/ZRDB/TM.py:_register
--On 13. Mai 2008 20:08:15 +0200 Dieter Maurer <[EMAIL PROTECTED]> wrote: Andreas Jung wrote at 2008-5-13 10:44 +0200: ... Is there any reason why TM._register is hiding exceptions ? Isn't the right approach for integrating a module with ZODB transactions using a ZODB DataManager? "Shared.DC.ZRDB.TM.TM" is the standard Zope[2] way to implement a ZODB DataManager. Nowadays you create a datamanager implementing IDataManager and join it with the current transaction. Shared.DC.ZRDB.TM.TM is pretty much old-old-old-style. Andreas pgpJa88lGWbkd.pgp Description: PGP signature ___ 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] Shared/DC/ZRDB/TM.py:_register
Andreas Jung wrote at 2008-5-13 10:44 +0200: > ... >> Is there any reason why TM._register is hiding exceptions ? > >Isn't the right approach for integrating a module with ZODB transactions >using a ZODB DataManager? "Shared.DC.ZRDB.TM.TM" is the standard Zope[2] way to implement a ZODB DataManager. And, as usual, "_register" should not hide exceptions. I.e., this is a bug. -- 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] Shared/DC/ZRDB/TM.py:_register
Vincent Pelletier wrote at 2008-5-13 10:36 +0200: > ... >To reproduce the problem: > - create a class inheriting from TM, and define a method calling register. > Add logs to abort and commit method to track transaction end. > - create a default error page which triggers a call to the method created > above. (this is equivalent to accessing some database from the error page, > for example) > Call it twice. > Most realistic case is using an instance surviving the transaction (a > global, or a persistant object) > - tigger an error in TPC (a raise in vote is the most realistic case) and get > the error page to render. The most obvious breakage I could see is when > trying to undo an non-undoable transaction (modify a script twice, undo the > oldest without undoing the newest) > >Here is what happens: > - first transaction (the "undo" in my example) raises in TPC, transaction is > marked as failed > - error message gets rendered in the same transaction (that's a ZPublisher > bug, but I think the problem "root" is hiding the failure) Formerly, the transaction was aborted before error handling. Someone convinced us that error handling should (of course) see the state the error happened and not a new clean state -- in order to be able to report about the errorneous state Therefore, it was changed. And I think, it was correct to change it. Another reason was also: should your error template need to run in a fresh transaction, then just abort the old one. If the transaction were aborted before error handling, then an error template with different requirements does not have a chance -- 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] Shared/DC/ZRDB/TM.py:_register
--On 13. Mai 2008 10:36:48 +0200 Vincent Pelletier <[EMAIL PROTECTED]> wrote: Hi. _register method of TM class in Shared/DC/ZRDB/TM.py hides all exceptions. This makes descendants of this class prone to get stuck in a "registered" state withtout any possibility of them to get commited. To reproduce the problem: - create a class inheriting from TM, and define a method calling register.Add logs to abort and commit method to track transaction end. - create a default error page which triggers a call to the method created above. (this is equivalent to accessing some database from the error page,for example) Call it twice. Most realistic case is using an instance surviving the transaction (a global, or a persistant object) - tigger an error in TPC (a raise in vote is the most realistic case) and getthe error page to render. The most obvious breakage I could see is whentrying to undo an non-undoable transaction (modify a script twice, undo theoldest without undoing the newest) Here is what happens: - first transaction (the "undo" in my example) raises in TPC, transaction ismarked as failed - error message gets rendered in the same transaction (that's a ZPublisherbug, but I think the problem "root" is hiding the failure) - error message tries to register itself to transaction manager: First try gets an exception (hidden in TM._register), but in the processtransaction's "_adapters" dict was modified. TM subclass instance is notmarked as registered. Second try, now there is no exception raised, because transaction's "join"is not called (because of _adapters content). Now the TM subclass instanceis marked as registered, but unknown to transaction (_adapters will not beused to commit). The variant with just one call to TM._register "only" causes the instance to be commited at next transaction using it. Is there any reason why TM._register is hiding exceptions ? Isn't the right approach for integrating a module with ZODB transactions using a ZODB DataManager? Andreas pgpBTR1J9YUMG.pgp Description: PGP signature ___ 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] Shared/DC/ZRDB/TM.py:_register
Hi. _register method of TM class in Shared/DC/ZRDB/TM.py hides all exceptions. This makes descendants of this class prone to get stuck in a "registered" state withtout any possibility of them to get commited. To reproduce the problem: - create a class inheriting from TM, and define a method calling register. Add logs to abort and commit method to track transaction end. - create a default error page which triggers a call to the method created above. (this is equivalent to accessing some database from the error page, for example) Call it twice. Most realistic case is using an instance surviving the transaction (a global, or a persistant object) - tigger an error in TPC (a raise in vote is the most realistic case) and get the error page to render. The most obvious breakage I could see is when trying to undo an non-undoable transaction (modify a script twice, undo the oldest without undoing the newest) Here is what happens: - first transaction (the "undo" in my example) raises in TPC, transaction is marked as failed - error message gets rendered in the same transaction (that's a ZPublisher bug, but I think the problem "root" is hiding the failure) - error message tries to register itself to transaction manager: First try gets an exception (hidden in TM._register), but in the process transaction's "_adapters" dict was modified. TM subclass instance is not marked as registered. Second try, now there is no exception raised, because transaction's "join" is not called (because of _adapters content). Now the TM subclass instance is marked as registered, but unknown to transaction (_adapters will not be used to commit). The variant with just one call to TM._register "only" causes the instance to be commited at next transaction using it. Is there any reason why TM._register is hiding exceptions ? -- Vincent Pelletier ___ 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
