Forum OpenACS Development: Re: db_transaction side effects

9: Re: db_transaction side effects (response to 1)

Posted by Dave Bauer on 06/01/06 06:34 PM

I suggest adding that flag is a accident waiting to happen.

Let's examine what the transactions are for in those procedures and decide if they belong in there or not.

Then if we decide they don't, the caller should wrap their code in a transaction if its necessary.

I admint I don't know if those transactions should be required or not. Perhaps someone has time to look at the calls to content::item::new and content::revision::new

I think currently content::item::new calls cotnent::revision::new in a transaction and there is a transaction in content::revision::new as well.

So this already can be way out of hand even without adding your own transaction around it.

10: Re: Re: db_transaction side effects (response to 9)

Posted by Jun Yamog on 06/01/06 11:04 PM

Hi Dave,

I think those db_transaction calls was inherited from the bcms procs. By design the transaction are required.

content::item::new creates a new content item and an initial content revision if there some data (eg. title, description, etc.) passed in. The initial thought was its likely you will want to create a content revision after creating a content item. Calling one proc at that time made sense to me.

content::revision::new initially may not have db_transacation, however it seems the latest one will require it since when the revision is created, its then updated with the tmp_filename

Anyway I hope this gives some historical account of those procs. I am looking at the code by webcvs only, so add some grain of salt. I do agree with you in not adding a flag, semantically I meant to have the db operation atomic similar to the plsql version. Hopefully a good solution is found and keeping the db operation atomic.

11: Re: Re: db_transaction side effects (response to 9)

Posted by Malte Sussdorff on 06/02/06 06:25 AM

Judging from your statements I see that having a flag for content::item::new and content::revision::new seems out of discussion.

Next question: Is it possible to tweak db_transaction to *not* execute yet another db_transaction if you are already in a db_transaction block. After all, when I am in a db_transaction I want to reroll everything if it fails at one point and not each transaction separately.

Maybe this would get around the behaviour I have seen sometimes (as in not reproduceable, but often enough for me to notice) that a content::item is generated even though the surrounding transaction fails.

Last but not least, before I change it, is there a general concern about keeping ::new or ::edit functions atomic in transactions. What I wonder is, shouldn't I add the flag to e.g. project::new, so when I call project::new in a transaction at least project::new will not execute yet another transaction. I never thought about this in detail, but at the moment I think it should not be much of an issue. But maybe I am wrong (after all you are adamant that it is a bad idea in content::item::new, so maybe the logic applied there applies everywhere else).

And I am aware that the "flag" is only a cure for the symptoms, not for the disease itself.

12: Re: db_transaction side effects (response to 11)

Posted by Nis Jørgensen on 06/02/06 10:29 AM

Malte, some of us have use cases for "real" nested transactions. Specifically, situations where the statement "After all, when I am in a db_transaction I want to reroll everything if it fails at one point and not each transaction separately." is false.

However, the Postgres branch of the db_api currently DOES NOT SUPPORT THESE. You can nest calls to db_transaction, but they don't actually do anything to the database. So it is not really clear what you are asking for with the "do not execute another db_transaction".

It seems you have an idea that using nested transactions are bad, something to be avoided. In my mind, they should be correctly implemented and used.

I am opposed to your idea of letting the caller decide whether he wants a transaction or not. This is an implementation decision, and shouldn't be exposed in the api.

13: Re: Re: db_transaction side effects (response to 12)

Posted by Malte Sussdorff on 06/02/06 11:18 AM

My assumption is that having multiple levels of db_transaction nested into each other currently stumbles over its own feet from time to time. Otherwise I could not explain the behaviour which started this thread. My suggestion was a "quick fix" to the problem derived from the fact that it happens at least 3-4 times a day in a production environment and I am not really keen on trying to fix the db_api where I do not even have a clue where to start.

So my assumption is we are talking about different things. I would be opposed to the idea of the caller deciding if a transaction should happen or not, could I be sure that having multiple nested transactions (and we are talking about 4 levels of procedure calls where each procedure has at least one db_transaction in them, sometimes more (e.g. if you call content::item::new and then content::revision::new)) does not make the system stumble.

I got the levels of transactions reduced in one instance and so far I did not get an error from it any more. I would be delighted if I had more scientific proof (e.g. a reproduceable test case) so one could maybe even look in postgres directly what is happening and why. But at the moment I don't.