Thread: Nested Transaction TODO list
Here's the promised list of things I think we still need to fix to
complete the nested-transactions project. I assume you have a private
todo list as well --- can we compare notes?
Cursors have a whole range of issues, as is already under discussion.
Present design for bufmgr and indexscan cleanup may be all wrong.
Still need to agree about externally visible behavior (a different stmt
than begin/commit for subxacts? What about savepoints?) Also, what about
exposing this functionality in plpgsql? Seems like we need some kind of
exception handling syntax to make this useful. What does Oracle do?
What about reporting transaction state/nesting level to client? I did not
like the GUC-variable approach in the original patch, partly on grounds of
efficiency and partly because I doubt it works under transaction-failure
conditions. I'm inclined to think we need a small protocol change.
Perhaps it would work to add an additional field to Z messages that is
only sent when nest level > 1.
Need to think about pg_locks view now that single backend may have multiple
XIDs --- do we need more info in pg_locks??
Why does postgres.c discriminate against START TRANSACTION where it
now allows BEGIN? Could simplify code by just allowing all
TransactionStmt types.
Does xact.c not allow a subtrans from TBLOCK_STARTED state? Should fix.
I wonder whether we shouldn't rename TopTransactionContext.
Any existing references to TopTransactionContext are more than likely wrong
given the way its semantics have been subtly changed. (I checked everything
in standard distro, but what about external PLs and user-written fns ...)
Check order of operations in subtransaction start/commit/abort. Is there
a good reason not to be fully consistent with top-level cases? Document
where there is.
trigger.c: not at all sure about the model for handling trigger firing
status. It looks like a subtrans could fire triggers that were pending
from an outer xact, which is dubious at best.
Comments in htup.h now need work; seems a shame to just throw 'em away
but I guess that's what we use CVS for.
Couldn't we simplify IsSubTransaction into a check on nest depth?
plpgsql: is it okay for simple_eval_estate to live throughout
the toplevel xact? subxact abort might release resources that
are still ref'd in estate, leading to trouble at top commit?
Maybe we need one simple_eval_estate per subxact ...
We should think about whether there's not a better way for
VACUUM FULL to work than session locks now.
xact_redo seems a bit optimistic about alignment? Might be OK, not sure.
Need to test on alignment-picky box such as HP...
Not sure about logic for OnCommitActions, take another look.
DONE/CANCELED logic for triggers looks fishy too, particularly test at
line 1946ff (2003ff in my committed patch)
TransactionIdIsInProgress needs work/review; comments are off for one
thing, and it seems *way* too inefficient. Note it should be possible to
skip subtrans search for sufficiently old xacts (hm ... couldn't we skip
sinval search too...)
catcache mechanism seems unacceptably inefficient as well.
Need to optimize on assumption that very few if any entries are pinned.
relcache same I think (although frequency with which entries will be
pinned is higher, and the total number of entries lower, so this is less
obviously sucky)
Most if not all of the uses of SubTransXidsHaveCommonAncestor should
probably go away, since it's rare to apply this test only once. Better
to fold the probe XID to its toplevel parent once instead of N times.
More generally, we have replaced cheap tests with expensive ones in
many places in tqual.c; this needs more thought.
Note TransactionIdIsCurrentTransactionId has become much looser.
Need to look at its uses ... (seems okay in a quick look but I'm not
totally convinced...)
XactLockTableWait --- annoying that it waits for top xact even when subtrans
has already aborted; possibly even a recipe for deadlock. Probably better
if we make subtransactions take out locks on their own XIDs, same as top
level. If these are held till commit/abort same as other locks, I think
we can simply revert XactLockTableWait to its prior state and it will do
just the right thing.
*why* can't you make a subtrans read only? (Probably just temporary
until GUC rollback is worked on?) See assign_transaction_read_only.
Need to make sure you can't loosen the constraint, though (no r/w
subxact of r/o parent), so we do need an assign hook to check that.
Seems like subtrans could/should be truncated much sooner than clog.
What we need here is a short-circuit test to avoid pursuing parent of
a subtrans older than, say, RecentGlobalXmin. If it isn't yet marked
committed then it must be aborted, no need to look at parent.
But think about race conditions and order of marking subxacts during
commit/abort.
[older version of same thought] Truncating subtrans log needs more thought
--- race conditions about when/whether a subxact is properly marked in
main clog as committed or aborted? I think we need to be able to assume
that subxact-commit state in clog actually means aborted, *without*
visiting subtrans, before some cutoff point. This is okay as long as the
cutoff point cannot advance past a main xact that is busy marking its
subtranses as committed. Check order of operations during commit.
Probably need to stay "in progress" in sinval until after all subtrans
marked.
Don't much like using lfirst_int etc for XIDs ... probably should
at least invent some macros lfirst_xid and so on.
Stuff that still needs examination:
lo_commit
localbuf refcounts (decide if we are changing bufmgr behavior, first)
EOXact callbacks API needs extension
namespace (temp namespace cleanup)
files
password file
SetReindexProcessing
regards, tom lane
On Sat, Jul 03, 2004 at 11:03:33AM -0400, Tom Lane wrote: > Here's the promised list of things I think we still need to fix to > complete the nested-transactions project. I assume you have a private > todo list as well --- can we compare notes? Hmm ... there are a lot of things in your list not in mine. The things that I have not in yours is pretty short: - fix SPI to work on functions not-in-xact-block with TransactionStmt (this is related to the item on subxacts under TBLOCK_STARTED) - fix large objects longevity Some comments: > Still need to agree about externally visible behavior (a different stmt > than begin/commit for subxacts? What about savepoints?) Also, what about > exposing this functionality in plpgsql? Seems like we need some kind of > exception handling syntax to make this useful. What does Oracle do? We should offer the savepoint syntax; seems easy to do. I think a lot of things are easier to do if we use a different syntax _and_ allow a subxact to start from TBLOCK_STARTED. > What about reporting transaction state/nesting level to client? I did not > like the GUC-variable approach in the original patch, partly on grounds of > efficiency and partly because I doubt it works under transaction-failure > conditions. I'm inclined to think we need a small protocol change. > Perhaps it would work to add an additional field to Z messages that is > only sent when nest level > 1. It's a shame to have to lose backwards compatibility. Why can't we use ParameterStatus? Perhaps having it as a GUC var was a bad idea, but we can do otherwise. > Why does postgres.c discriminate against START TRANSACTION where it > now allows BEGIN? Could simplify code by just allowing all > TransactionStmt types. Oversight. > I wonder whether we shouldn't rename TopTransactionContext. > Any existing references to TopTransactionContext are more than likely wrong > given the way its semantics have been subtly changed. (I checked everything > in standard distro, but what about external PLs and user-written fns ...) We need to think about this and interaction with EOXact callbacks -- a non-subxact-aware function could easily break if called inside a subxact. > TransactionIdIsInProgress needs work/review; comments are off for one > thing, and it seems *way* too inefficient. Note it should be possible to > skip subtrans search for sufficiently old xacts (hm ... couldn't we skip > sinval search too...) Can we use a cutoff like RecentGlobalXmin here? -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "No reniegues de lo que alguna vez creíste"
Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> On Sat, Jul 03, 2004 at 11:03:33AM -0400, Tom Lane wrote:
>> What about reporting transaction state/nesting level to client? I did not
>> like the GUC-variable approach in the original patch, partly on grounds of
>> efficiency and partly because I doubt it works under transaction-failure
>> conditions. I'm inclined to think we need a small protocol change.
>> Perhaps it would work to add an additional field to Z messages that is
>> only sent when nest level > 1.
> It's a shame to have to lose backwards compatibility.
I don't like using ParameterStatus because it's not designed for dealing
with values that may change many times in a single query. Also it sends
strings, which this really isn't.
I haven't looked at JDBC, but at least in the libpq code, what we could
safely do is extend the existing no transaction/in transaction/in failed
transaction field to provide a five-way distinction: those three cases
plus in subtransaction/in failed subtransaction. You could not directly
tell the depth of your subxact from this, but do you need to?
regards, tom lane
On Sat, 3 Jul 2004, Tom Lane wrote: > trigger.c: not at all sure about the model for handling trigger firing > status. It looks like a subtrans could fire triggers that were pending > from an outer xact, which is dubious at best. Well, SET CONSTRAINTS potentially needs to check the state of any outstanding constraints whose state changes from deferred to immediate. I don't think we can say that it sets a constraint to immediate, but doesn't check outstanding instances because they were from an outer transaction (unless the constraint state reverted on commit which seems really odd).
On Sat, Jul 03, 2004 at 11:03:33AM -0400, Tom Lane wrote: > Why does postgres.c discriminate against START TRANSACTION where it > now allows BEGIN? Could simplify code by just allowing all > TransactionStmt types. Why does START have a different Node from BEGIN anyway? This seems to be a leftover from when people thought they should behave differently. They are the same now, so there's no point in distinguishing them, or is it? -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Saca el libro que tu religión considere como el indicado para encontrar la oración que traiga paz a tu alma. Luego rebootea el computador y ve si funciona" (Carlos Duclós)
Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> Why does START have a different Node from BEGIN anyway? This seems to
> be a leftover from when people thought they should behave differently.
> They are the same now, so there's no point in distinguishing them, or is it?
[shrug...] I'd counsel leaving this as-is. We've practically always
regretted it when we made the parser discard information about what
the user typed. For instance, I was just reminded yesterday that we
really ought to distinguish SortClauses created due to user ORDER BY
clauses from those created because the parser silently added 'em.
regards, tom lane
On Sat, 03 Jul 2004 17:40:23 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Alvaro Herrera <alvherre@dcc.uchile.cl> writes: > > Why does START have a different Node from BEGIN anyway? This seems to > > be a leftover from when people thought they should behave differently. > > They are the same now, so there's no point in distinguishing them, or is it? > > [shrug...] I'd counsel leaving this as-is. We've practically always > regretted it when we made the parser discard information about what > the user typed. For instance, I was just reminded yesterday that we > really ought to distinguish SortClauses created due to user ORDER BY > clauses from those created because the parser silently added 'em. How about simply documenting semantic equivalences, and making them somewhat more explicit to the user? Regards Haroldo -- Por favor registre haroldo.stenger@gmail.com como mi nueva y unica direccion de correo en lugar de la vieja hstenger@adinet.com.uy Please, record haroldo.stenger@gmail.com as my new and unique email address instead of ye old hstenger@adinet.com.uy Gracias. Thankyou.
Tom Lane wrote: > I don't like using ParameterStatus because it's not designed for dealing > with values that may change many times in a single query. Also it sends > strings, which this really isn't. What about including the new nesting level in the SUBBEGIN/SUBCOMMIT/SUBABORT CommandStatus string? Yes, it's still a string for a numeric nesting level, but that's also how we pass numeric data such as insert/update rowcounts. > I haven't looked at JDBC, but at least in the libpq code, what we could > safely do is extend the existing no transaction/in transaction/in failed > transaction field to provide a five-way distinction: those three cases > plus in subtransaction/in failed subtransaction. You could not directly > tell the depth of your subxact from this, but do you need to? This will break the existing JDBC driver in nonobvious ways: the current code silently ignores unhandled transaction states in ReadyForQuery, so you could conceivably end up in situations where the driver thinks you're outside a transaction when you're actually inside a subtransaction, and issues spurious BEGINs. It's simple enough to handle the new states, but it *is* an incompatible protocol change. Please bump the protocol version if you do make this change. And if you're going to bump the protocol version, I have some other changes I'd like to see at the same time :) If named SAVEPOINT syntax (along the lines of what Oracle has) is available in 7.5, and there is a mechanism to COMMIT/ROLLBACK the top-level transaction directly, the JDBC driver doesn't actually need to know anything about subtransactions to correctly implement JDBC's commit, rollback, & savepoint interface -- except perhaps to deal with the case where the application rolls back a subtransaction which invalidates an earlier SAVEPOINT. Having the nesting level available will probably make savepoint support a bit easier but it's not vital. If we don't have COMMIT/ROLLBACK of top-level transactions, then the JDBC driver needs to know the current nesting level so it knows how many subtransactions to deal with when a top-level commit/rollback is requested via the JDBC API. I'd actually prefer to see plain COMMIT and ROLLBACK continue to operate on the top-level transaction, since doing anything else is going to break pre-7.5 clients (such as older JDBC drivers) that assume that you're entirely outside a transaction after COMMIT/ROLLBACK. If we don't have SAVEPOINT syntax, then the JDBC driver needs to know the current nesting level so it can track which (client-maintained) savepoints are still valid in the face of the application doing subtransaction work itself. The only other thing the JDBC driver uses the transaction state for is to work out when BEGINs are needed when JDBC-level autocommit is off, and when to allow transaction isolation level changes. As far as I can see, "in subtransaction" is equivalent to "in top-level transaction" for both of those cases. So overall, knowing that you're in a subtransaction without knowing the nesting level does not seem very useful.. -O
Oliver Jowett <oliver@opencloud.com> writes:
> Tom Lane wrote:
>> I haven't looked at JDBC, but at least in the libpq code, what we could
>> safely do is extend the existing no transaction/in transaction/in failed
>> transaction field to provide a five-way distinction: those three cases
>> plus in subtransaction/in failed subtransaction.
> This will break the existing JDBC driver in nonobvious ways: the current
> code silently ignores unhandled transaction states in ReadyForQuery,
Drat. Scratch that plan then. (Still, silently ignoring unrecognized
states probably wasn't a good idea for the JDBC code...)
regards, tom lane
Tom Lane wrote: > Oliver Jowett <oliver@opencloud.com> writes: > >>Tom Lane wrote: >> >>>I haven't looked at JDBC, but at least in the libpq code, what we could >>>safely do is extend the existing no transaction/in transaction/in failed >>>transaction field to provide a five-way distinction: those three cases >>>plus in subtransaction/in failed subtransaction. > > >>This will break the existing JDBC driver in nonobvious ways: the current >>code silently ignores unhandled transaction states in ReadyForQuery, > > > Drat. Scratch that plan then. (Still, silently ignoring unrecognized > states probably wasn't a good idea for the JDBC code...) True, but the alternative (screaming and yelling) would also have broken, just more obviously. Actually, thinking about it, that behaviour only changed recently, and from memory the older code completely ignored the transaction state in ReadyForQuery. The new driver probably hasn't spread too far yet. I'll sort out a patch so the driver breaks more obviously if it gets something unexpected. I still don't think that knowing you're in a subtransaction is very useful unless you also know the nesting level. -O
On Sat, Jul 03, 2004 at 11:12:56PM -0400, Tom Lane wrote: > Oliver Jowett <oliver@opencloud.com> writes: > > Tom Lane wrote: > >> I haven't looked at JDBC, but at least in the libpq code, what we could > >> safely do is extend the existing no transaction/in transaction/in failed > >> transaction field to provide a five-way distinction: those three cases > >> plus in subtransaction/in failed subtransaction. > > > This will break the existing JDBC driver in nonobvious ways: the current > > code silently ignores unhandled transaction states in ReadyForQuery, > > Drat. Scratch that plan then. (Still, silently ignoring unrecognized > states probably wasn't a good idea for the JDBC code...) What about using the command tag of SUBBEGIN &c ? -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) Hi! I'm a .signature virus! cp me into your .signature file to help me spread!
Tom Lane wrote: > Still need to agree about externally visible behavior (a different stmt > than begin/commit for subxacts? What about savepoints?) Also, what about > exposing this functionality in plpgsql? Seems like we need some kind of > exception handling syntax to make this useful. What does Oracle do? As I just mentioned in another thread, whatever the syntax for nested transactions I'd like to see plain COMMIT/ABORT/ROLLBACK always affect the top-level transaction. Oracle appears to have: SAVEPOINT savepointname ROLLBACK [WORK] [TO [SAVEPOINT] savepointname] You can issue SAVEPOINT with the same name while the old savepoint is valid, and the name will be moved. Rolling back to a savepoint does not invalidate that savepoint, i.e. you can roll back to a savepoint multiple times. One generalization of this to nested transactions would be: SUBBEGIN [transactionname] SUBCOMMIT [transactionname] SUBABORT [transactionname] SUBBEGIN outside an explicit transaction block works like BEGIN. Active transactions may have names. SUBBEGIN with a name associates the name with the new transaction; if the name is already in use, it's also removed from the old transaction. Alternatively we could only look at the most-deeply-nested transaction with a given name when specifying transactions by name. That would make savepoint behaviour slightly different to Oracle (Oracle could see a savepoint as invalid that we consider valid), but it looks like it'd make things a bit easier for procedural languages as functions can't accidentally trash a name "belonging" to your caller so long as they resolve all transactions they start. SUBCOMMIT or SUBABORT work on the current transaction level (if no name is specified) or all transactions down to (and including) the named transaction level if a name is given. "SAVEPOINT savepointname" becomes an alias for "SUBBEGIN savepointname". "ROLLBACK TO [SAVEPOINT] savepointname" becomes an alias for "SUBABORT savepointname; SUBBEGIN savepointname". We could spell SUBBEGIN and friends differently -- is it better to add more syntax to the existing transaction manipulation commands along the lines of "BEGIN [NESTED] [TRANSACTION|WORK] [transactionname]", "ROLLBACK [NESTED] [TRANSACTION|WORK] [transactionname]" etc? Any comments? -O
On Mon, Jul 05, 2004 at 03:38:13PM +1200, Oliver Jowett wrote: > As I just mentioned in another thread, whatever the syntax for nested > transactions I'd like to see plain COMMIT/ABORT/ROLLBACK always affect > the top-level transaction. > > Oracle appears to have: > > SAVEPOINT savepointname > ROLLBACK [WORK] [TO [SAVEPOINT] savepointname] Right ... this is also what the standard defines. A slight difference from your description is that if one issues ROLLBACK TO savepointname everything from the savepoint is rolled back, but the savepoint itself is kept, so later I can roll back to it again. > One generalization of this to nested transactions would be: > > SUBBEGIN [transactionname] > SUBCOMMIT [transactionname] > SUBABORT [transactionname] The only departure from the SAVEPOINT syntax is that you are able to "subcommit" a savepoint. Not sure how useful that is ... > Active transactions may have names. SUBBEGIN with a name associates the > name with the new transaction; if the name is already in use, it's also > removed from the old transaction. Alternatively we could only look at > the most-deeply-nested transaction with a given name when specifying > transactions by name. Interesting idea ... it's also easier to implement. Also maybe it can be used to simplify life for PL handlers aborting a function. > We could spell SUBBEGIN and friends differently -- is it better to add > more syntax to the existing transaction manipulation commands along the > lines of "BEGIN [NESTED] [TRANSACTION|WORK] [transactionname]", > "ROLLBACK [NESTED] [TRANSACTION|WORK] [transactionname]" etc? Not sure. I already implemented SUBBEGIN. How does that work for everyone? I don't see much value in overloading BEGIN/ROLLBACK. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "El día que dejes de cambiar dejarás de vivir"
Alvaro Herrera wrote: >>One generalization of this to nested transactions would be: >> >> SUBBEGIN [transactionname] >> SUBCOMMIT [transactionname] >> SUBABORT [transactionname] > > > The only departure from the SAVEPOINT syntax is that you are able to > "subcommit" a savepoint. Not sure how useful that is ... One thing SUBCOMMIT [name] does allow is discarding savepoints / named txns without rolling back their changes. That might be useful if we allow nesting of names, e.g.: SAVEPOINT save1 -- do work #1 SAVEPOINT save1 -- hides the earlier SAVEPOINT -- do work #2 SAVEPOINT save2 -- do work #3 SUBCOMMIT save1 -- provisionally commits #2 and #3 to enclosing txn -- do work #4 ROLLBACKTO save1 -- rolls back #1, #2, #3, #4 Other than that.. I assume we want SUBBEGIN/SUBCOMMIT/SUBABORT regardless of how we do savepoints. Since savepoints are a subset of what you can do with nested transactions, it seems appropriate that SUBBEGIN etc. can do everything that savepoints can -- i.e. naming of savepoints/transactions. And then SUBCOMMIT [name] is just there for completeness. -O
On Sat, Jul 03, 2004 at 11:03:33AM -0400, Tom Lane wrote: > TransactionIdIsInProgress needs work/review; comments are off for one > thing, and it seems *way* too inefficient. Note it should be possible to > skip subtrans search for sufficiently old xacts (hm ... couldn't we skip > sinval search too...) I am looking at this now ... the first thing I did was stamp at the start of the function if (TransactionIdPrecedes(xid, RecentGlobalXmin)) return false; So we don't need to check pg_subtrans (nor the PGPROC array) for any transaction that is too old. Now, I'm looking at adding the array of cached Xids and I think that maybe this is not the cure to all the performance problems introduced. With the cached Xid array we can return quickly for a transaction that is part of any of the current subtransaction trees, but there's no way to know about "negative hits"! So any time a Xid that's not part of any transaction tree is sought, we'd have to revert to pg_subtrans. This includes an aborted subtransaction of a current transaction tree. So how about adding two arrays to PGPROC: the cached Xid array (for subcommitted subxacts) and another which would hold aborted Xids? We would store Xids of aborted subxacts of the current transaction. (Maybe we could store Xids of previous transactions of this backend too?) Instead of the second array, we could have a global TransactionId array which would hold past transactions and aborted subtransactions for all backends. At AbortTransaction() and AbortSubTransaction() we would save the Xid there, using a round-robin scheme. What do you think? -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "No single strategy is always right (Unless the boss says so)" (Larry Wall)
Tom Lane wrote: > Alvaro Herrera <alvherre@dcc.uchile.cl> writes: > > Why does START have a different Node from BEGIN anyway? This seems to > > be a leftover from when people thought they should behave differently. > > They are the same now, so there's no point in distinguishing them, or is it? > > [shrug...] I'd counsel leaving this as-is. We've practically always > regretted it when we made the parser discard information about what > the user typed. For instance, I was just reminded yesterday that we > really ought to distinguish SortClauses created due to user ORDER BY > clauses from those created because the parser silently added 'em. What information are we loosing by having START and BEGIN use the same nodes? Knowing what keyword they used to start the transaction? Seems that would only be important if we wanted them to behave differently, which we don't, I think. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
On Sat, Jul 03, 2004 at 11:03:33AM -0400, Tom Lane wrote: > than begin/commit for subxacts? What about savepoints?) Also, what about > exposing this functionality in plpgsql? Seems like we need some kind of > exception handling syntax to make this useful. What does Oracle do? Oracle uses savepoints: SAVEPOINT savepointname; creates a savepoint or shifts existing savepoint of the same name; ROLLBACK TO savepointname; rolls back to savepoint (more verbose syntax also available); The syntax of handling exceptions is (in PL/SQL): BEGIN some code, for example a bunch of SQL commands;EXCEPTION WHEN nameofexception THEN handle the exception,maybe ROLLBACK;END; There are predefined exceptions like INVALID_NUMBER, NO_DATA_FOUND, ZERO_DIVIDE, or OTHERS. -- ------------------------------------------------------------------------Honza Pazdziora | adelton@fi.muni.cz | http://www.fi.muni.cz/~adelton/.project:Perl, mod_perl, DBI, Oracle, large Web systems, XML/XSL, ... Only self-confidentpeople can be simple.
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> [shrug...] I'd counsel leaving this as-is.
> What information are we loosing by having START and BEGIN use the same
> nodes? Knowing what keyword they used to start the transaction?
Exactly.
> Seems that would only be important if we wanted them to behave
> differently, which we don't, I think.
Whether we want them to behave differently or not, we need to preserve
the difference. The prior cases where the parser smashed two different
inputs into the same parse tree have all been "because it doesn't
matter", and sure enough we've usually eventually decided it did matter.
regards, tom lane