Forum OpenACS Q&A: dp_process and nested transactions...

Collapse
1: dp_process and nested transactions...
Posted by Don Baccus on 04/12/00 03:03 AM
Thought I'd point out a "gotcha" with the 3.2 data pipelining module.
I noticed when looking through the (virgin, unported) intranet
sources while doing some contract work today that there's at least one
construct like this:

ns_db dml "begin"

dp_process $some_stuff

ns db dml "insert into something values(...some more stuff...)"

ns_db dml "end".

dp_process also uses begin/end to start a transaction.  However,
Postgres doesn't support nested transactions.  I'm too busy at the
moment to look at the ported version, but if this wasn't caught in
this (or other cases) the second "begin" will cause a warning (?) and
the nested "end" will end the transaction, with the outer "end" giving
a warning (for sure).

It might appear to work but the atomicity intended for the actions
would be demolished by virtue of the first, nested "end" causing the
transaction to end.

My suggestion is to add a flag to dp_process telling it whether or not
it is in a transaction, and to avoid doing the "begin/end" in this
case.

I haven't checked the Oracle driver to see if the code actually works
for Oracle, either, I'm a bit suspicious that it might not, it would
have to count begin/end nesting and only set autocommit mode when and
end returns the nest to zero.  I could do something like this in the
driver but it seems a bit odd in the general, non-ACS AOLserver/PG
case...

Comments?  Dan, did you catch this when you ported intranet and work
around it?

Fortunately, at the moment dp_process is rarely used but I suspect it
will be used very often in the future.

Collapse
2: Response to dp_process and nested transactions... (response to 1)
Posted by Don Baccus on 04/12/00 03:23 AM
Well, golly-gee-whiz, it doesn't work with the Oracle driver, either,  this is yet again another bug in the ACS.  Yes, I've actually tested this with a real script.

aD hasn't noticed because the haven't had a disk die at precisely the right point in time, I guess.  Cool!  Oracle abuse!

Collapse
3: Response to dp_process and nested transactions... (response to 1)
Posted by Daniel Wickstrom on 04/12/00 08:59 AM
I never looked at these db_process constuct's much, because there didn't seem to be any problem with them, but I just grepped through the intranet sources, and I see about 4 instances of this particular construct.

I've added a -in_transaction flag to db_process and changed the corresponding intranet sources.

Thanks for the heads up.

Collapse
4: Response to dp_process and nested transactions... (response to 1)
Posted by Don Baccus on 04/12/00 03:14 PM
I've started a thread in web/db, too...there are other problems, Janine Sisk e-mailed me a construct like this:

with_transaction { ... ns_db "begin/end transaction" ... }

Same problem wearing a different disguise.

Jin Choi sez they plan to have the Oracle driver spit out an error when it hits a begin within a transaction, in this case at least they'll catch the errors.

Anyway, I think the parameter-to-dp_process notion is a fine idea.  Let's try to track aD on this one, I think there's a chance they'll adopt the same notion.

Collapse
5: Response to dp_process and nested transactions... (response to 1)
Posted by Don Baccus on 04/13/00 01:04 AM
OK, got the final word regarding dp_process:

The begin/end transaction statements were supposed to have been removed before release, with the responsibility for establishing the transaction left to the caller.

Which is probably the way it should be, so one can start using the "with_transaction { .... }" paradigm with dp_process as well as direct  insert/update statements.

We should track this...

Collapse
6: Response to dp_process and nested transactions... (response to 1)
Posted by Daniel Wickstrom on 04/13/00 09:34 AM
O.k. I'll go ahead and update the db_process proc and the intranet sources.
Collapse
7: Response to dp_process and nested transactions... (response to 1)
Posted by Daniel Wickstrom on 04/13/00 09:55 AM
Actually since I'm changing dp_process, I'll go ahead and check all of the source code to and modify dp_process calls so that they are wrapped in the with_transaction { ... } construct.
Collapse
8: Response to dp_process and nested transactions... (response to 1)
Posted by Don Baccus on 04/14/00 04:48 PM
If you take a look at the thread at web/db, you'll see I've pointed out another whole area where transaction-busting seems to take place, i.e. the use of utility routines like ad_categorize_row inside transactions.  In fact, the document suggest using multiple calls to ad_categorize_row in some cases, which the reader will naturally put inside a transaction, which naturally busts the transaction.

The paradigm I'm using myself is to build up a list of dml statements, then execute them all within a transaction at the top level:

set dml [list]

hammer_the_db $dml parameters...

hammer_it_again $dml parameters...

with_transaction $db {
  foreach dml_stmt $dml {
      ns_db dml $db $dml_stmt
  }
} {...error handling...}

retaining the convenience of the utility routines while not screwing up transaction semantics by nesting "begin/end transaction" statements.

This could be made even simpler with a proc that takes a list of dml statements, called something like:

transact $db $dml {...error handling...}

where dml was built up as in my earlier example.

Comments?

I don't know if the aD folks are taking my reports seriously, though they will if they actually implement Jin's notion of  having the driver flag an error if a "begin" dml statement is issued from within a transaction - they'll get errors all over the friggin' place if they do this.