Forum OpenACS Development: db_transaction side effects

Collapse
Posted by Malte Sussdorff on
I have a question. Is it possible that db_transaction is a reason for strange behaviour with regards to the content:: procedures? Here is what I experienced:

- occasionally, when calling content::revision::new within a db_transaction, I will get a server error with acs_objects_context_object_un constraint violated. This is independent from the table I insert into (this happend to me with two different tables). The last two calls was iv::offer::edit and translate::freelancer-booking::edit (so yes, both times it was the edit function). Both are called within a db_transaction.

- When calling content::item::move from the move.tcl page I usually get nothing in return, though the content_item is created. Again, this is wrapped in a db_transaction.

My knowledge of db_transaction is limited, so before going down the road and looking in the wrong place, maybe someone can enlighten me an tell me how, short of a double click which never happened, content::revision::new could throw an error like the one described above.

Collapse
Posted by Dave Bauer on
Malte,

The best way to find out is to write an automated test that reproduces the condidtions you are finding in an isolated way so we can pinpoint the problem.

Once we have a reproducable test, it i easier to work together on a fix.

Collapse
Posted by Malte Sussdorff on
Sad thing is, the word occasionally should imply that I can't reproduce the error conditions. It just happens without any trace of the why. Hitting reload works sometimes. Sometimes you end up with multiple revisions. Sometimes the item is there, but the revision is not.

But entering the same data as the same user 3 minutes later works perfectly fine.

One of my assumtions is that we might have a strange race condition when we have nested db_transactions. Example:

db_transaction {
invoice::edit

.... more stuff that needs execution ....
}

where invoice::edit has

db_transaction {
content::revision::new

.. more stuff that needs execution ...
}

content::revision::new again has

db_transaction {
if {[string equal "" $revision_id]} {
set revision_id [db_nextval "acs_object_id_seq"]
}
db_dml insert_revision $query_text
update_content -revision_id $revision_id -content $content
}
}

and at the query text it breaks. But as i said, I can't tell you why and when. And I am no guru with regards to postgres to be able to say what could trigger the db_dml statement to be executed twice, resulting in the error message of the unique constraint (or when could an insert also violate a unique constraint).

Nis, this is PG 8.0.6 we are talking about.

Collapse
Posted by Nis Jørgensen on
Where seeing the same error with Greenpeace/Planet 2.

I would be interested in knowing which version of PG you are using - we are on 7.4.7, hoping to upgrade soon.

Collapse
Posted by Nis Jørgensen on
s/Where/We are/
Collapse
Posted by Dave Bauer on
I wonder if

1) these nested transactions are what we really want to happen when we call TCL apis like this.

Originally you'd just call the pl/sql directly from within your on db_transaction.

Since in PG in you have nested transactions you can't really do anything useful with the information if the content::revision::new call fails, in the outer transaction, maybe we need a way to not use a transaction in content::revisioN::new if is it called within a transaction.

Maybe Don is around and can comment on that idea :)

Collapse
Posted by Nis Jørgensen on
I assume you mean "Since in PG you _don't_ have nested transactions".

I just found out that postgres 8 supports savepoints, which can be used to implement nested transactions (in fact this seems to be the standard compliant way of doing so).

/Nis

Collapse
Posted by Malte Sussdorff on
I would propose of adding a flag "no_transaction_p" to both content::revision::new and content::item::new so we can say that they should not be called with a transaction.
Collapse
Posted by Dave Bauer on
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.

Collapse
Posted by Jun Yamog on
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.

Collapse
Posted by Malte Sussdorff on
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.

Collapse
Posted by Nis Jørgensen on
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.

Collapse
Posted by Malte Sussdorff on
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.