pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
SSI atomic commit
26+ messages / 5 participants
[nested] [flat]

Commits from this thread:
  2023-03-09 65e388d418 af397c6c27 d811d74be3 ae632f7a31 e30fd09424 d1c0f81e72  Fix race in SERIALIZABLE READ ONLY.
* SSI atomic commit
@ 2011-07-05 17:03  Kevin Grittner <Kevin.Grittner@wicourts.gov>
  0 siblings, 2 replies; 26+ messages in thread

From: Kevin Grittner @ 2011-07-05 17:03 UTC (permalink / raw)
  To: pgsql-hackers

In reviewing the 2PC changes mentioned in a separate post, both Dan
and I realized that these were dependent on the assumption that
SSI's commitSeqNo is assigned in the order in which the transactions
became visible.  There is a race condition such that this is not
necessarily true.  It is a very narrow race condition, which would
come up very rarely in practice, but Murphy's Law being what it is,
I think we need to consider it a bug and fix it.
 
We considered a fix which would be contained within predicate.c code
and operate by making pessimistic assumptions, so that no false
negatives occurred.  The reason we didn't go that way is that the
code would be very convoluted and fragile.  The attached patch just
makes it atomic in a very direct way, and adjusts the predicate.c
code to use the right tests in the right places.  We were careful
not to add any LW locking to the path that a normal transaction
without an XID is terminating, since there had obviously been
significant work put into keeping locks out of that code path.
 
Dan and I collaborated on this code over the holiday weekend, and
are in agreement that this is the right route.  I know it's only six
days before beta3, but I'm hopeful that someone can review this for
commit in time to make it into that beta.
 
This patch applies over the top of the fix for the 2PC permutation
problems.
 
I apologize for having found this bug so late in the release cycle.
 
-Kevin

Attachments:

ssi-atomic-commit.patch (text/plain, 19.3K) download *** a/src/backend/access/transam/twophase.c --- b/src/backend/access/transam/twophase.c *************** *** 1298,1314 **** FinishPreparedTransaction(const char *gid, bool isCommit) * callbacks will release the locks the transaction held. */ if (isCommit) RecordTransactionCommitPrepared(xid, hdr->nsubxacts, children, hdr->ncommitrels, commitrels, hdr->ninvalmsgs, invalmsgs, hdr->initfileinval); else RecordTransactionAbortPrepared(xid, hdr->nsubxacts, children, hdr->nabortrels, abortrels); ! ! ProcArrayRemove(&gxact->proc, latestXid); /* * In case we fail while running the callbacks, mark the gxact invalid so --- 1298,1318 ---- * callbacks will release the locks the transaction held. */ if (isCommit) + { RecordTransactionCommitPrepared(xid, hdr->nsubxacts, children, hdr->ncommitrels, commitrels, hdr->ninvalmsgs, invalmsgs, hdr->initfileinval); + CommitRemove2PC(&gxact->proc, latestXid); + } else + { RecordTransactionAbortPrepared(xid, hdr->nsubxacts, children, hdr->nabortrels, abortrels); ! ProcArrayRemove(&gxact->proc, latestXid); ! } /* * In case we fail while running the callbacks, mark the gxact invalid so *** a/src/backend/access/transam/xact.c --- b/src/backend/access/transam/xact.c *************** *** 1878,1884 **** CommitTransaction(void) * must be done _before_ releasing locks we hold and _after_ * RecordTransactionCommit. */ ! ProcArrayEndTransaction(MyProc, latestXid); /* * This is all post-commit cleanup. Note that if an error is raised here, --- 1878,1884 ---- * must be done _before_ releasing locks we hold and _after_ * RecordTransactionCommit. */ ! CommitEndTransaction(MyProc, latestXid); /* * This is all post-commit cleanup. Note that if an error is raised here, *** a/src/backend/storage/lmgr/predicate.c --- b/src/backend/storage/lmgr/predicate.c *************** *** 120,125 **** --- 120,128 ---- * - The same lock protects a target, all locks on that target, and * the linked list of locks on the target.. * - When more than one is needed, acquire in ascending order. + & - There are times when this lock must be held concurrently with + * ProcArrayLock (acquired in called functions, not directly). + * In such cases this lock must be acquired first. * * SerializableXactHashLock * - Protects both PredXact and SerializableXidHash. *************** *** 158,163 **** --- 161,168 ---- * PredicateLockPageCombine(Relation relation, BlockNumber oldblkno, * BlockNumber newblkno); * TransferPredicateLocksToHeapRelation(Relation relation) + * CommitEndTransaction(PGPROC *proc, TransactionId latestXid) + * CommitRemove2PC(PGPROC *proc, TransactionId latestXid) * ReleasePredicateLocks(bool isCommit) * * conflict detection (may also trigger rollback) *************** *** 252,261 **** * The PREPARED flag remains set after commit, so SxactIsCommitted * implies SxactIsPrepared. */ ! #define SxactIsCommitted(sxact) (((sxact)->flags & SXACT_FLAG_COMMITTED) != 0) ! #define SxactIsPrepared(sxact) (((sxact)->flags & SXACT_FLAG_PREPARED) != 0) ! #define SxactIsRolledBack(sxact) (((sxact)->flags & SXACT_FLAG_ROLLED_BACK) != 0) #define SxactIsDoomed(sxact) (((sxact)->flags & SXACT_FLAG_DOOMED) != 0) #define SxactIsReadOnly(sxact) (((sxact)->flags & SXACT_FLAG_READ_ONLY) != 0) #define SxactHasSummaryConflictIn(sxact) (((sxact)->flags & SXACT_FLAG_SUMMARY_CONFLICT_IN) != 0) #define SxactHasSummaryConflictOut(sxact) (((sxact)->flags & SXACT_FLAG_SUMMARY_CONFLICT_OUT) != 0) --- 257,267 ---- * The PREPARED flag remains set after commit, so SxactIsCommitted * implies SxactIsPrepared. */ ! #define SxactIsCommitted(sxact) ((sxact)->commitSeqNo != InvalidSerCommitSeqNo) ! #define SxactIsDoomed(sxact) (((sxact)->flags & SXACT_FLAG_DOOMED) != 0) + #define SxactIsPrepared(sxact) (((sxact)->flags & SXACT_FLAG_PREPARED) != 0) + #define SxactIsReleased(sxact) (((sxact)->flags & SXACT_FLAG_RELEASED) != 0) #define SxactIsReadOnly(sxact) (((sxact)->flags & SXACT_FLAG_READ_ONLY) != 0) #define SxactHasSummaryConflictIn(sxact) (((sxact)->flags & SXACT_FLAG_SUMMARY_CONFLICT_IN) != 0) #define SxactHasSummaryConflictOut(sxact) (((sxact)->flags & SXACT_FLAG_SUMMARY_CONFLICT_OUT) != 0) *************** *** 1194,1200 **** InitPredicateLocks(void) PredXact->OldCommittedSxact->topXid = InvalidTransactionId; PredXact->OldCommittedSxact->finishedBefore = InvalidTransactionId; PredXact->OldCommittedSxact->xmin = InvalidTransactionId; ! PredXact->OldCommittedSxact->flags = SXACT_FLAG_COMMITTED; PredXact->OldCommittedSxact->pid = 0; } /* This never changes, so let's keep a local copy. */ --- 1200,1207 ---- PredXact->OldCommittedSxact->topXid = InvalidTransactionId; PredXact->OldCommittedSxact->finishedBefore = InvalidTransactionId; PredXact->OldCommittedSxact->xmin = InvalidTransactionId; ! PredXact->OldCommittedSxact->flags = ! SXACT_FLAG_PREPARED | SXACT_FLAG_RELEASED; PredXact->OldCommittedSxact->pid = 0; } /* This never changes, so let's keep a local copy. */ *************** *** 1669,1676 **** RegisterSerializableTransactionInt(Snapshot snapshot) othersxact != NULL; othersxact = NextPredXact(othersxact)) { ! if (!SxactIsOnFinishedList(othersxact) && ! !SxactIsReadOnly(othersxact)) { SetPossibleUnsafeConflict(sxact, othersxact); } --- 1676,1684 ---- othersxact != NULL; othersxact = NextPredXact(othersxact)) { ! if (!SxactIsCommitted(othersxact) ! && !SxactIsDoomed(othersxact) ! && !SxactIsReadOnly(othersxact)) { SetPossibleUnsafeConflict(sxact, othersxact); } *************** *** 3052,3059 **** SetNewSxactGlobalXmin(void) for (sxact = FirstPredXact(); sxact != NULL; sxact = NextPredXact(sxact)) { ! if (!SxactIsRolledBack(sxact) ! && !SxactIsCommitted(sxact) && sxact != OldCommittedSxact) { Assert(sxact->xmin != InvalidTransactionId); --- 3060,3066 ---- for (sxact = FirstPredXact(); sxact != NULL; sxact = NextPredXact(sxact)) { ! if (!SxactIsReleased(sxact) && sxact != OldCommittedSxact) { Assert(sxact->xmin != InvalidTransactionId); *************** *** 3074,3079 **** SetNewSxactGlobalXmin(void) --- 3081,3167 ---- } /* + * CommitEndTransaction + * + * For correct SERIALIZABLE semantics, the commitSeqNo must appear to be set + * atomically with the work of the transaction becoming visible to other + * transactions. + * + * Since a transaction without an xid has not made any changes which will be + * visible to other transactions, it doesn't need such treatment. These + * transactions will be flagged as committed and assigned a commitSeqNo in + * ReleasePredicateLocks(). This is after the actual commit, but the timing + * for these transactions isn't critical. Since they are explicitly or + * implicitly READ ONLY, it is the start time of the transaction which is + * critical for correct detection of serialization failures; the commit + * information is only really needed for cleanup purposes. + */ + void + CommitEndTransaction(PGPROC *proc, TransactionId latestXid) + { + if (MySerializableXact == InvalidSerializableXact + || !TransactionIdIsValid(latestXid)) + { + ProcArrayEndTransaction(proc, latestXid); + return; + } + + LWLockAcquire(SerializableXactHashLock, LW_EXCLUSIVE); + + ProcArrayEndTransaction(proc, latestXid); + + MySerializableXact->commitSeqNo = ++(PredXact->LastSxactCommitSeqNo); + + LWLockRelease(SerializableXactHashLock); + } + + /* + * CommitRemove2PC + * + * Like CommitEndTransaction, except for a prepared transaction which is being + * committed. + */ + void + CommitRemove2PC(PGPROC *proc, TransactionId latestXid) + { + SERIALIZABLEXIDTAG sxidtag; + SERIALIZABLEXID *sxid; + + if (!TransactionIdIsValid(latestXid)) + { + ProcArrayRemove(proc, latestXid); + return; + } + + LWLockAcquire(SerializableXactHashLock, LW_EXCLUSIVE); + + sxidtag.xid = latestXid; + sxid = (SERIALIZABLEXID *) + hash_search(SerializableXidHash, &sxidtag, HASH_FIND, NULL); + + /* + * xid will not be found if it wasn't a serializable transaction. In that + * case, make the transaction's work visible while not holding + * SerializableXactHashLock. + */ + if (sxid == NULL) + { + LWLockRelease(SerializableXactHashLock); + ProcArrayRemove(proc, latestXid); + return; + } + + ProcArrayRemove(proc, latestXid); + + MySerializableXact = sxid->myXact; + MyXactDidWrite = true; /* conservatively assume that we wrote + * something */ + MySerializableXact->commitSeqNo = ++(PredXact->LastSxactCommitSeqNo); + + LWLockRelease(SerializableXactHashLock); + } + + /* * ReleasePredicateLocks * * Releases predicate locks based on completion of the current transaction, *************** *** 3116,3126 **** ReleasePredicateLocks(bool isCommit) return; } - Assert(!isCommit || SxactIsPrepared(MySerializableXact)); - Assert(!isCommit || !SxactIsDoomed(MySerializableXact)); - Assert(!SxactIsCommitted(MySerializableXact)); - Assert(!SxactIsRolledBack(MySerializableXact)); - /* may not be serializable during COMMIT/ROLLBACK PREPARED */ if (MySerializableXact->pid != 0) Assert(IsolationIsSerializable()); --- 3204,3209 ---- *************** *** 3132,3137 **** ReleasePredicateLocks(bool isCommit) --- 3215,3224 ---- LWLockAcquire(SerializableXactHashLock, LW_EXCLUSIVE); + Assert(!SxactIsReleased(MySerializableXact)); + Assert(!isCommit || SxactIsPrepared(MySerializableXact)); + Assert(!isCommit || !SxactIsDoomed(MySerializableXact)); + /* * We don't hold XidGenLock lock here, assuming that TransactionId is * atomic! *************** *** 3145,3158 **** ReleasePredicateLocks(bool isCommit) */ MySerializableXact->finishedBefore = ShmemVariableCache->nextXid; /* * If it's not a commit it's a rollback, and we can clear our locks * immediately. */ if (isCommit) { ! MySerializableXact->flags |= SXACT_FLAG_COMMITTED; ! MySerializableXact->commitSeqNo = ++(PredXact->LastSxactCommitSeqNo); /* Recognize implicit read-only transaction (commit without write). */ if (!MyXactDidWrite) MySerializableXact->flags |= SXACT_FLAG_READ_ONLY; --- 3232,3249 ---- */ MySerializableXact->finishedBefore = ShmemVariableCache->nextXid; + MySerializableXact->flags |= SXACT_FLAG_RELEASED; + /* * If it's not a commit it's a rollback, and we can clear our locks * immediately. */ if (isCommit) { ! if (MySerializableXact->commitSeqNo == InvalidSerCommitSeqNo) ! { ! MySerializableXact->commitSeqNo = ++(PredXact->LastSxactCommitSeqNo); ! } /* Recognize implicit read-only transaction (commit without write). */ if (!MyXactDidWrite) MySerializableXact->flags |= SXACT_FLAG_READ_ONLY; *************** *** 3165,3178 **** ReleasePredicateLocks(bool isCommit) * other transactions that conflict with it. Note that this flag might * already be set, if another backend marked this transaction for * abort. - * - * The ROLLED_BACK flag further indicates that ReleasePredicateLocks - * has been called, and so the SerializableXact is eligible for - * cleanup. This means it should not be considered when calculating - * SxactGlobalXmin. */ MySerializableXact->flags |= SXACT_FLAG_DOOMED; ! MySerializableXact->flags |= SXACT_FLAG_ROLLED_BACK; /* * If the transaction was previously prepared, but is now failing due * to a ROLLBACK PREPARED or (hopefully very rare) error after the --- 3256,3264 ---- * other transactions that conflict with it. Note that this flag might * already be set, if another backend marked this transaction for * abort. */ MySerializableXact->flags |= SXACT_FLAG_DOOMED; ! /* * If the transaction was previously prepared, but is now failing due * to a ROLLBACK PREPARED or (hopefully very rare) error after the *************** *** 3260,3274 **** ReleasePredicateLocks(bool isCommit) && !SxactIsReadOnly(MySerializableXact) && SxactIsCommitted(conflict->sxactIn)) { ! if ((MySerializableXact->flags & SXACT_FLAG_CONFLICT_OUT) == 0 || conflict->sxactIn->commitSeqNo < MySerializableXact->SeqNo.earliestOutConflictCommit) MySerializableXact->SeqNo.earliestOutConflictCommit = conflict->sxactIn->commitSeqNo; MySerializableXact->flags |= SXACT_FLAG_CONFLICT_OUT; } ! if (!isCommit ! || SxactIsCommitted(conflict->sxactIn) ! || (conflict->sxactIn->SeqNo.lastCommitBeforeSnapshot >= PredXact->LastSxactCommitSeqNo)) ReleaseRWConflict(conflict); conflict = nextConflict; --- 3346,3358 ---- && !SxactIsReadOnly(MySerializableXact) && SxactIsCommitted(conflict->sxactIn)) { ! if (!SxactHasConflictOut(MySerializableXact) || conflict->sxactIn->commitSeqNo < MySerializableXact->SeqNo.earliestOutConflictCommit) MySerializableXact->SeqNo.earliestOutConflictCommit = conflict->sxactIn->commitSeqNo; MySerializableXact->flags |= SXACT_FLAG_CONFLICT_OUT; } ! if (!isCommit || SxactIsCommitted(conflict->sxactIn)) ReleaseRWConflict(conflict); conflict = nextConflict; *************** *** 3290,3296 **** ReleasePredicateLocks(bool isCommit) offsetof(RWConflictData, inLink)); if (!isCommit ! || SxactIsCommitted(conflict->sxactOut) || SxactIsReadOnly(conflict->sxactOut)) ReleaseRWConflict(conflict); --- 3374,3380 ---- offsetof(RWConflictData, inLink)); if (!isCommit ! || SxactIsReleased(conflict->sxactOut) || SxactIsReadOnly(conflict->sxactOut)) ReleaseRWConflict(conflict); *************** *** 3556,3562 **** ReleaseOneSerializableXact(SERIALIZABLEXACT *sxact, bool partial, nextConflict; Assert(sxact != NULL); ! Assert(SxactIsRolledBack(sxact) || SxactIsCommitted(sxact)); Assert(LWLockHeldByMe(SerializableFinishedListLock)); /* --- 3640,3646 ---- nextConflict; Assert(sxact != NULL); ! Assert(SxactIsReleased(sxact)); Assert(LWLockHeldByMe(SerializableFinishedListLock)); /* *************** *** 4702,4722 **** PredicateLockTwoPhaseFinish(TransactionId xid, bool isCommit) SERIALIZABLEXID *sxid; SERIALIZABLEXIDTAG sxidtag; ! sxidtag.xid = xid; ! LWLockAcquire(SerializableXactHashLock, LW_SHARED); ! sxid = (SERIALIZABLEXID *) ! hash_search(SerializableXidHash, &sxidtag, HASH_FIND, NULL); ! LWLockRelease(SerializableXactHashLock); ! /* xid will not be found if it wasn't a serializable transaction */ ! if (sxid == NULL) ! return; ! /* Release its locks */ ! MySerializableXact = sxid->myXact; ! MyXactDidWrite = true; /* conservatively assume that we wrote * something */ ReleasePredicateLocks(isCommit); } --- 4786,4810 ---- SERIALIZABLEXID *sxid; SERIALIZABLEXIDTAG sxidtag; ! if (MySerializableXact == InvalidSerializableXact) ! { ! sxidtag.xid = xid; ! LWLockAcquire(SerializableXactHashLock, LW_SHARED); ! sxid = (SERIALIZABLEXID *) ! hash_search(SerializableXidHash, &sxidtag, HASH_FIND, NULL); ! LWLockRelease(SerializableXactHashLock); ! /* xid will not be found if it wasn't a serializable transaction */ ! if (sxid == NULL) ! return; ! /* Release its locks */ ! MySerializableXact = sxid->myXact; ! MyXactDidWrite = true; /* conservatively assume that we wrote * something */ + } + ReleasePredicateLocks(isCommit); } *** a/src/include/storage/predicate.h --- b/src/include/storage/predicate.h *************** *** 14,19 **** --- 14,20 ---- #ifndef PREDICATE_H #define PREDICATE_H + #include "storage/proc.h" #include "utils/relcache.h" #include "utils/snapshot.h" *************** *** 50,55 **** extern void PredicateLockTuple(Relation relation, HeapTuple tuple, Snapshot snap --- 51,58 ---- extern void PredicateLockPageSplit(Relation relation, BlockNumber oldblkno, BlockNumber newblkno); extern void PredicateLockPageCombine(Relation relation, BlockNumber oldblkno, BlockNumber newblkno); extern void TransferPredicateLocksToHeapRelation(Relation relation); + extern void CommitEndTransaction(PGPROC *proc, TransactionId latestXid); + extern void CommitRemove2PC(PGPROC *proc, TransactionId latestXid); extern void ReleasePredicateLocks(bool isCommit); /* conflict detection (may also trigger rollback) */ *** a/src/include/storage/predicate_internals.h --- b/src/include/storage/predicate_internals.h *************** *** 90,111 **** typedef struct SERIALIZABLEXACT int pid; /* pid of associated process */ } SERIALIZABLEXACT; ! #define SXACT_FLAG_COMMITTED 0x00000001 /* already committed */ #define SXACT_FLAG_PREPARED 0x00000002 /* about to commit */ ! #define SXACT_FLAG_ROLLED_BACK 0x00000004 /* already rolled back */ ! #define SXACT_FLAG_DOOMED 0x00000008 /* will roll back */ /* * The following flag actually means that the flagged transaction has a * conflict out *to a transaction which committed ahead of it*. It's hard * to get that into a name of a reasonable length. */ ! #define SXACT_FLAG_CONFLICT_OUT 0x00000010 ! #define SXACT_FLAG_READ_ONLY 0x00000020 ! #define SXACT_FLAG_DEFERRABLE_WAITING 0x00000040 ! #define SXACT_FLAG_RO_SAFE 0x00000080 ! #define SXACT_FLAG_RO_UNSAFE 0x00000100 ! #define SXACT_FLAG_SUMMARY_CONFLICT_IN 0x00000200 ! #define SXACT_FLAG_SUMMARY_CONFLICT_OUT 0x00000400 /* * The following types are used to provide an ad hoc list for holding --- 90,110 ---- int pid; /* pid of associated process */ } SERIALIZABLEXACT; ! #define SXACT_FLAG_DOOMED 0x00000001 /* will roll back */ #define SXACT_FLAG_PREPARED 0x00000002 /* about to commit */ ! #define SXACT_FLAG_RELEASED 0x00000004 /* OK to clean up */ /* * The following flag actually means that the flagged transaction has a * conflict out *to a transaction which committed ahead of it*. It's hard * to get that into a name of a reasonable length. */ ! #define SXACT_FLAG_CONFLICT_OUT 0x00000008 ! #define SXACT_FLAG_READ_ONLY 0x00000010 ! #define SXACT_FLAG_DEFERRABLE_WAITING 0x00000020 ! #define SXACT_FLAG_RO_SAFE 0x00000040 ! #define SXACT_FLAG_RO_UNSAFE 0x00000080 ! #define SXACT_FLAG_SUMMARY_CONFLICT_IN 0x00000100 ! #define SXACT_FLAG_SUMMARY_CONFLICT_OUT 0x00000200 /* * The following types are used to provide an ad hoc list for holding *** a/src/test/regress/expected/prepared_xacts.out --- b/src/test/regress/expected/prepared_xacts.out *************** *** 134,140 **** SELECT * FROM pxtest1; -- This should fail, because the two transactions have a write-skew anomaly INSERT INTO pxtest1 VALUES ('fff'); ERROR: could not serialize access due to read/write dependencies among transactions ! DETAIL: Cancelled on identification as a pivot, during write. HINT: The transaction might succeed if retried. PREPARE TRANSACTION 'foo5'; SELECT gid FROM pg_prepared_xacts; --- 134,140 ---- -- This should fail, because the two transactions have a write-skew anomaly INSERT INTO pxtest1 VALUES ('fff'); ERROR: could not serialize access due to read/write dependencies among transactions ! DETAIL: Canceled on identification as a pivot, during write. HINT: The transaction might succeed if retried. PREPARE TRANSACTION 'foo5'; SELECT gid FROM pg_prepared_xacts;
^ permalink raw reply [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-05 18:01  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Kevin Grittner <Kevin.Grittner@wicourts.gov>
  1 sibling, 2 replies; 26+ messages in thread

From: Heikki Linnakangas @ 2011-07-05 18:01 UTC (permalink / raw)
  To: Kevin Grittner <Kevin.Grittner@wicourts.gov>; +Cc: pgsql-hackers

On 05.07.2011 20:03, Kevin Grittner wrote:
> In reviewing the 2PC changes mentioned in a separate post, both Dan
> and I realized that these were dependent on the assumption that
> SSI's commitSeqNo is assigned in the order in which the transactions
> became visible.  There is a race condition such that this is not
> necessarily true.  It is a very narrow race condition, which would
> come up very rarely in practice, but Murphy's Law being what it is,
> I think we need to consider it a bug and fix it.
>
> We considered a fix which would be contained within predicate.c code
> and operate by making pessimistic assumptions, so that no false
> negatives occurred.  The reason we didn't go that way is that the
> code would be very convoluted and fragile.  The attached patch just
> makes it atomic in a very direct way, and adjusts the predicate.c
> code to use the right tests in the right places.  We were careful
> not to add any LW locking to the path that a normal transaction
> without an XID is terminating, since there had obviously been
> significant work put into keeping locks out of that code path.

Hmm, I think it would be simpler to decide that instead of 
SerializableXactHashLock, you must hold ProcArrayLock to access 
LastSxactCommitSeqNo, and move the assignment of commitSeqNo to 
ProcArrayTransaction(). It's probably easiest to move 
LastSxactCommitSeqno to ShmemVariableCache too. There's a few places 
that would then need to acquire ProcArrayLock to read 
LastSxactCommitSeqno, but I feel it might still be much simpler that way.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-05 18:15  Kevin Grittner <Kevin.Grittner@wicourts.gov>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  1 sibling, 1 reply; 26+ messages in thread

From: Kevin Grittner @ 2011-07-05 18:15 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: pgsql-hackers

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:
 
> Hmm, I think it would be simpler to decide that instead of 
> SerializableXactHashLock, you must hold ProcArrayLock to access 
> LastSxactCommitSeqNo, and move the assignment of commitSeqNo to 
> ProcArrayTransaction(). It's probably easiest to move 
> LastSxactCommitSeqno to ShmemVariableCache too. There's a few
> places that would then need to acquire ProcArrayLock to read 
> LastSxactCommitSeqno, but I feel it might still be much simpler
> that way.
 
We considered that.  I think the biggest problem was that when there
is no XID it wouldn't be covered by the lock on assignment.  We
couldn't see a good way to increment and assign the value without LW
lock coverage, and we didn't want to add LW locking to that code
path.  If you can see a way around that issue, I agree it would be
simpler.
 
-Kevin



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-05 18:16  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  1 sibling, 1 reply; 26+ messages in thread

From: Tom Lane @ 2011-07-05 18:16 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Kevin Grittner <Kevin.Grittner@wicourts.gov>; pgsql-hackers

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> Hmm, I think it would be simpler to decide that instead of 
> SerializableXactHashLock, you must hold ProcArrayLock to access 
> LastSxactCommitSeqNo, and move the assignment of commitSeqNo to 
> ProcArrayTransaction(). It's probably easiest to move 
> LastSxactCommitSeqno to ShmemVariableCache too. There's a few places 
> that would then need to acquire ProcArrayLock to read 
> LastSxactCommitSeqno, but I feel it might still be much simpler that way.

Yeah ... this patch creats the need to hold both
SerializableXactHashLock and ProcArrayLock during transaction commit,
which is a bit scary from a deadlock-risk perspective, and not pleasant
from the concurrency standpoint either.  It'd be better to push some
functionality into the procarray code.

			regards, tom lane



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-05 18:35  Kevin Grittner <Kevin.Grittner@wicourts.gov>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 26+ messages in thread

From: Kevin Grittner @ 2011-07-05 18:35 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers

Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
>> Hmm, I think it would be simpler to decide that instead of 
>> SerializableXactHashLock, you must hold ProcArrayLock to access 
>> LastSxactCommitSeqNo, and move the assignment of commitSeqNo to 
>> ProcArrayTransaction(). It's probably easiest to move 
>> LastSxactCommitSeqno to ShmemVariableCache too. There's a few
>> places that would then need to acquire ProcArrayLock to read 
>> LastSxactCommitSeqno, but I feel it might still be much simpler
>> that way.
> 
> Yeah ... this patch creats the need to hold both
> SerializableXactHashLock and ProcArrayLock during transaction
> commit, which is a bit scary from a deadlock-risk perspective,
 
If I remember correctly, we already do some calls to functions which
acquire ProcArrayLock while holding SerializableXactHashLock, in
order to acquire a snapshot with the right timing.  Regardless of
what we do on transaction completion, we either need to document
that in the code comments, or do some major surgery at the last
minute, which is always scary.
 
> and not pleasant from the concurrency standpoint either.
 
On the bright side, both locks are not held at the same time unless
the transaction isolation level is serializable.
 
> It'd be better to push some functionality into the procarray code.
 
That's easily done if we don't mind taking out a ProcArrayLock
during completion of a transaction which has no XID, if only long
enough to increment a uint64 in shared memory, and then stash the
value -- somewhere -- so that SSI code can find and use it.  We
thought it was best to avoid that so there wasn't any impact on
non-serializable transactions.
 
-Kevin



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-05 19:17  Robert Haas <robertmhaas@gmail.com>
  parent: Kevin Grittner <Kevin.Grittner@wicourts.gov>
  0 siblings, 2 replies; 26+ messages in thread

From: Robert Haas @ 2011-07-05 19:17 UTC (permalink / raw)
  To: Kevin Grittner <Kevin.Grittner@wicourts.gov>; +Cc: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers

On Tue, Jul 5, 2011 at 2:35 PM, Kevin Grittner
<Kevin.Grittner@wicourts.gov> wrote:
>> It'd be better to push some functionality into the procarray code.
>
> That's easily done if we don't mind taking out a ProcArrayLock
> during completion of a transaction which has no XID, if only long
> enough to increment a uint64 in shared memory, and then stash the
> value -- somewhere -- so that SSI code can find and use it.

That sure sounds scary from a scalability perspective.  If we can
piggyback on an existing ProcArrayLock acquisition, fine, but
additional ProcArrayLock acquisitions when SSI isn't even being used
sound like a real bad idea to me.  I doubt you'll notice much of a
performance regression in the current code, but if/when we fix the
lock manager bottlenecks that my fastlock and lazy vxid lock patches
are intended to correct, then I suspect it's going to matter quite a
bit.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-05 19:30  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Robert Haas <robertmhaas@gmail.com>
  1 sibling, 1 reply; 26+ messages in thread

From: Tom Lane @ 2011-07-05 19:30 UTC (permalink / raw)
  To: Robert Haas <robertmhaas@gmail.com>; +Cc: Kevin Grittner <Kevin.Grittner@wicourts.gov>; Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Jul 5, 2011 at 2:35 PM, Kevin Grittner
> <Kevin.Grittner@wicourts.gov> wrote:
>> That's easily done if we don't mind taking out a ProcArrayLock
>> during completion of a transaction which has no XID, if only long
>> enough to increment a uint64 in shared memory, and then stash the
>> value -- somewhere -- so that SSI code can find and use it.

> That sure sounds scary from a scalability perspective.  If we can
> piggyback on an existing ProcArrayLock acquisition, fine, but
> additional ProcArrayLock acquisitions when SSI isn't even being used
> sound like a real bad idea to me.

Isn't SSI *already* forcing a new acquisition of an LWLock during
commits of read-only transactions that aren't using SSI?  Perhaps
there's a bit less contention on SerializableXactHashLock than on
ProcArrayLock, but it's not obvious that the current situation is
a lot better than this would be.

			regards, tom lane



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-05 19:34  Kevin Grittner <Kevin.Grittner@wicourts.gov>
  parent: Robert Haas <robertmhaas@gmail.com>
  1 sibling, 0 replies; 26+ messages in thread

From: Kevin Grittner @ 2011-07-05 19:34 UTC (permalink / raw)
  To: Robert Haas <robertmhaas@gmail.com>; +Cc: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; pgsql-hackers; Tom Lane <tgl@sss.pgh.pa.us>

Robert Haas <robertmhaas@gmail.com> wrote:
> On Tue, Jul 5, 2011 at 2:35 PM, Kevin Grittner
> <Kevin.Grittner@wicourts.gov> wrote:
>>> It'd be better to push some functionality into the procarray
>>> code.
>>
>> That's easily done if we don't mind taking out a ProcArrayLock
>> during completion of a transaction which has no XID, if only long
>> enough to increment a uint64 in shared memory, and then stash the
>> value -- somewhere -- so that SSI code can find and use it.
> 
> That sure sounds scary from a scalability perspective.  If we can
> piggyback on an existing ProcArrayLock acquisition, fine, but
> additional ProcArrayLock acquisitions when SSI isn't even being
> used sound like a real bad idea to me.  I doubt you'll notice much
> of a performance regression in the current code, but if/when we
> fix the lock manager bottlenecks that my fastlock and lazy vxid
> lock patches are intended to correct, then I suspect it's going to
> matter quite a bit.
 
Just to be clear, the patch as submitted does *not* acquire a
ProcArrayLock lock for completion of a transaction which has no XID.
What you quoted from me was explaining what would seem to be
required (unless Dan and I missed something) to simplify the patch
as Tom and Heikki proposed.  We saw that approach and its
simplicity, but shied away from it because of the locking issue and
its potential performance impact.
 
We took the route we did in this fix patch to avoid such issues. 
I'm not so sure that the impact on a load of many short read-only
transactions would be so benign as things stand.  It seemed to me
that one of the reasons to avoid *getting* an XID was to avoid
acquiring ProcArrayLock on transaction completion.  The way we did
this patch may indeed slow serializable transactions more than the
alternative; but from the beginning I thought the ground rules were
that SSI had to have no significant impact on those who didn't
choose to use it.
 
I suspect that most of the 9.2 work on SSI will be for improved
performance (including in that, as I do, a reduction in the
percentage of false positive serialization failures).  Tuning this
should go on that list.  It may well benefit from using some of the
techniques you've been working on.
 
-Kevin



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-05 19:40  Kevin Grittner <Kevin.Grittner@wicourts.gov>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 26+ messages in thread

From: Kevin Grittner @ 2011-07-05 19:40 UTC (permalink / raw)
  To: Robert Haas <robertmhaas@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; pgsql-hackers

Tom Lane <tgl@sss.pgh.pa.us> wrote:
 
> Isn't SSI *already* forcing a new acquisition of an LWLock during
> commits of read-only transactions that aren't using SSI?
 
During COMMIT PREPARED there is one.  We could avoid that by storing
the transaction isolation level in the persistent data for a
prepared statement, but that seems inappropriate for 9.1 at this
point, and it's hard to be sure that would be a net win.  Otherwise
I don't *think* there's an extra LW lock for a non-serializable
transaction (whether or not read-only). Do you see one I'm not
remembering?
 
-Kevin



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-05 19:54  Dan Ports <drkp@csail.mit.edu>
  parent: Kevin Grittner <Kevin.Grittner@wicourts.gov>
  0 siblings, 0 replies; 26+ messages in thread

From: Dan Ports @ 2011-07-05 19:54 UTC (permalink / raw)
  To: Kevin Grittner <Kevin.Grittner@wicourts.gov>; +Cc: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; pgsql-hackers

On Tue, Jul 05, 2011 at 01:15:13PM -0500, Kevin Grittner wrote:
> Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:
>  
> > Hmm, I think it would be simpler to decide that instead of 
> > SerializableXactHashLock, you must hold ProcArrayLock to access 
> > LastSxactCommitSeqNo, and move the assignment of commitSeqNo to 
> > ProcArrayTransaction(). It's probably easiest to move 
> > LastSxactCommitSeqno to ShmemVariableCache too. There's a few
> > places that would then need to acquire ProcArrayLock to read 
> > LastSxactCommitSeqno, but I feel it might still be much simpler
> > that way.
>  
> We considered that.  I think the biggest problem was that when there
> is no XID it wouldn't be covered by the lock on assignment.

One other issue is that after the sequence number is assigned, it still
needs to be stored in MySerializableXact->commitSeqNo. Modifying that
does require taking SerializableXactHashLock.

With the proposed patch, assigning the next commitSeqNo and storing it
in MySerializableXact happen atomically. That makes it possible to say
that a transaction that has a commitSeqNo must have committed before
one that doesn't. If the two steps are separated, that isn't true: two
transactions might get their commitSeqNos in one order and make them
visible in the other. We should be able to deal with that, but it will
make some of the commit ordering checks more complicated.

Dan

-- 
Dan R. K. Ports              MIT CSAIL                http://drkp.net/



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 15:45  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Kevin Grittner <Kevin.Grittner@wicourts.gov>
  1 sibling, 1 reply; 26+ messages in thread

From: Heikki Linnakangas @ 2011-07-07 15:45 UTC (permalink / raw)
  To: Kevin Grittner <Kevin.Grittner@wicourts.gov>; +Cc: pgsql-hackers

On 05.07.2011 20:03, Kevin Grittner wrote:
> In reviewing the 2PC changes mentioned in a separate post, both Dan
> and I realized that these were dependent on the assumption that
> SSI's commitSeqNo is assigned in the order in which the transactions
> became visible.

This comment in the patch actually suggests a stronger requirement:

> + * For correct SERIALIZABLE semantics, the commitSeqNo must appear to be set
> + * atomically with the work of the transaction becoming visible to other
> + * transactions.

So, is it enough for the commitSeqNos to be set in the order the 
transactions become visible to others? I'm assuming 'yes' for now, as 
the approach being discussed to assign commitSeqNo in 
ProcArrayEndTransaction() without also holding SerializableXactHashLock 
is not going to work otherwise, and if I understood correctly you didn't 
see any correctness issue with that. Please shout if I'm missing something.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 16:41  Kevin Grittner <Kevin.Grittner@wicourts.gov>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  0 siblings, 1 reply; 26+ messages in thread

From: Kevin Grittner @ 2011-07-07 16:41 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: pgsql-hackers

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:
> On 05.07.2011 20:03, Kevin Grittner wrote:
>> In reviewing the 2PC changes mentioned in a separate post, both
>> Dan and I realized that these were dependent on the assumption
>> that SSI's commitSeqNo is assigned in the order in which the
>> transactions became visible.
> 
> This comment in the patch actually suggests a stronger
> requirement:
> 
>> + * For correct SERIALIZABLE semantics, the commitSeqNo must
>>     appear to be set
>> + * atomically with the work of the transaction becoming visible
>>     to other
>> + * transactions.
> 
> So, is it enough for the commitSeqNos to be set in the order the 
> transactions become visible to others? I'm assuming 'yes' for now,
> as the approach being discussed to assign commitSeqNo in 
> ProcArrayEndTransaction() without also holding
> SerializableXactHashLock is not going to work otherwise, and if I
> understood correctly you didn't see any correctness issue with
> that. Please shout if I'm missing something.
 
Hmm.  The more strict semantics are much easier to reason about and
ensure correctness.  I think that the looser semantics can be made
to work, but there needs to be more fussy logic in the SSI code to
deal with the fact that we don't know whether the visibility change
has occurred.  Really what pushed us to the patch using the stricter
semantics was how much the discussion of how to cover the edge
conditions with the looser semantics made our heads spin.  Anything
that confusing seems more prone to subtle bugs.
 
If people really think that route is better I can put a patch
together so that the two approaches can be compared side-by-side. 
If we go that way, it seemed that maybe we should move
LastSxactCommitSeqNo to VariableCacheData, right below
latestCompletedXid.  We need some way to communicate that the
commitSeqNo has been set -- probably a volatile boolean in local
memory, and we need to acquire ProcArrayLock in some places we
currently don't within the SSI code to get at the value.  Off-hand,
I don't see how that can be done without holding
SerializableXactHashLock while grabbing ProcArrayLock; and if we
need to do that to grab the assigned value, I'm not sure how much
we're buying.  Do you see some way to avoid that?
 
The other thing we need to do if we go with the looser semantics is
be pessimistic -- in all tests for dangerous structures we need to
assume that any transaction which is prepared *may* also be
committed, and may have committed before any other transaction which
is prepared or committed.  It would be good to put some bounds on
this.  I guess any monotonically increasing number in the system
which can be grabbed just before the commit would do.
 
Unless you've come up with some clever approach we missed, I fear
that a patch based on the looser semantics will be bigger and more
fragile.
 
Do you agree that the patch Dan and I posted *does not add any LW
locking* to a normal (not prepared) transaction if it either has no
XID or is not SERIALIZABLE?  And that ProcArrayLock is not held
during COMMIT PREPARED or ROLLBACK PREPARED unless the transaction
is SERIALIZABLE?  I'm a bit concerned that we're headed down this
other path because people aren't clear about these facts.
 
-Kevin



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 18:50  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Kevin Grittner <Kevin.Grittner@wicourts.gov>
  0 siblings, 2 replies; 26+ messages in thread

From: Heikki Linnakangas @ 2011-07-07 18:50 UTC (permalink / raw)
  To: Kevin Grittner <Kevin.Grittner@wicourts.gov>; +Cc: pgsql-hackers

On 07.07.2011 19:41, Kevin Grittner wrote:
> Heikki Linnakangas<heikki.linnakangas@enterprisedb.com>  wrote:
>> On 05.07.2011 20:03, Kevin Grittner wrote:
>>> In reviewing the 2PC changes mentioned in a separate post, both
>>> Dan and I realized that these were dependent on the assumption
>>> that SSI's commitSeqNo is assigned in the order in which the
>>> transactions became visible.
>>
>> This comment in the patch actually suggests a stronger
>> requirement:
>>
>>> + * For correct SERIALIZABLE semantics, the commitSeqNo must
>>>      appear to be set
>>> + * atomically with the work of the transaction becoming visible
>>>      to other
>>> + * transactions.
>>
>> So, is it enough for the commitSeqNos to be set in the order the
>> transactions become visible to others? I'm assuming 'yes' for now,
>> as the approach being discussed to assign commitSeqNo in
>> ProcArrayEndTransaction() without also holding
>> SerializableXactHashLock is not going to work otherwise, and if I
>> understood correctly you didn't see any correctness issue with
>> that. Please shout if I'm missing something.
>
> Hmm.  The more strict semantics are much easier to reason about and
> ensure correctness.

True.

>  I think that the looser semantics can be made
> to work, but there needs to be more fussy logic in the SSI code to
> deal with the fact that we don't know whether the visibility change
> has occurred.  Really what pushed us to the patch using the stricter
> semantics was how much the discussion of how to cover the edge
> conditions with the looser semantics made our heads spin.  Anything
> that confusing seems more prone to subtle bugs.

Let's step back and analyse a bit closer what goes wrong with the 
current semantics. The problem is that commitSeqNo is assigned too late, 
sometime after the transaction has become visible to others.

Looking at the comparisons that we do with commitSeqNos, they all have 
conservative direction that we can err to, when in doubt. So here's an idea:

Let's have two sequence numbers for each transaction: prepareSeqNo and 
commitSeqNo. prepareSeqNo is assigned when a transaction is prepared (in 
PreCommit_CheckForSerializableConflicts), and commitSeqNo is assigned 
when it's committed (in ReleasePredicateLocks). They are both assigned 
from one counter, LastSxactCommitSeqNo, so that is advanced twice per 
transaction, and prepareSeqNo is always smaller than commitSeqNo for a 
transaction. Modify operations that currently use commitSeqNo to use 
either prepareSeqNo or commitSeqNo, so that we err on the safe side.

That yields a much smaller patch (attached). How does this look to you, 
am I missing anything?

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

Attachments:

track-prepareseqno-1.patch (text/x-diff, 3.6K) download diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index f0c8ee4..134a0a0 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -1184,6 +1184,7 @@ InitPredicateLocks(void) } PredXact->OldCommittedSxact = CreatePredXact(); SetInvalidVirtualTransactionId(PredXact->OldCommittedSxact->vxid); + PredXact->OldCommittedSxact->prepareSeqNo = 0; PredXact->OldCommittedSxact->commitSeqNo = 0; PredXact->OldCommittedSxact->SeqNo.lastCommitBeforeSnapshot = 0; SHMQueueInit(&PredXact->OldCommittedSxact->outConflicts); @@ -1644,6 +1645,7 @@ RegisterSerializableTransactionInt(Snapshot snapshot) /* Initialize the structure. */ sxact->vxid = vxid; sxact->SeqNo.lastCommitBeforeSnapshot = PredXact->LastSxactCommitSeqNo; + sxact->prepareSeqNo = InvalidSerCommitSeqNo; sxact->commitSeqNo = InvalidSerCommitSeqNo; SHMQueueInit(&(sxact->outConflicts)); SHMQueueInit(&(sxact->inConflicts)); @@ -4401,18 +4403,13 @@ OnConflict_CheckForSerializationFailure(const SERIALIZABLEXACT *reader, { SERIALIZABLEXACT *t2 = conflict->sxactIn; - /* - * Note that if T2 is merely prepared but not yet committed, we - * rely on t->commitSeqNo being InvalidSerCommitSeqNo, which is - * larger than any valid commit sequence number. - */ if (SxactIsPrepared(t2) && (!SxactIsCommitted(reader) - || t2->commitSeqNo <= reader->commitSeqNo) + || t2->prepareSeqNo <= reader->commitSeqNo) && (!SxactIsCommitted(writer) - || t2->commitSeqNo <= writer->commitSeqNo) + || t2->prepareSeqNo <= writer->commitSeqNo) && (!SxactIsReadOnly(reader) - || t2->commitSeqNo <= reader->SeqNo.lastCommitBeforeSnapshot)) + || t2->prepareSeqNo <= reader->SeqNo.lastCommitBeforeSnapshot)) { failure = true; break; @@ -4453,17 +4450,11 @@ OnConflict_CheckForSerializationFailure(const SERIALIZABLEXACT *reader, { SERIALIZABLEXACT *t0 = conflict->sxactOut; - /* - * Note that if the writer is merely prepared but not yet - * committed, we rely on writer->commitSeqNo being - * InvalidSerCommitSeqNo, which is larger than any valid commit - * sequence number. - */ if (!SxactIsDoomed(t0) && (!SxactIsCommitted(t0) - || t0->commitSeqNo >= writer->commitSeqNo) + || t0->commitSeqNo >= writer->prepareSeqNo) && (!SxactIsReadOnly(t0) - || t0->SeqNo.lastCommitBeforeSnapshot >= writer->commitSeqNo)) + || t0->SeqNo.lastCommitBeforeSnapshot >= writer->prepareSeqNo)) { failure = true; break; @@ -4602,6 +4593,7 @@ PreCommit_CheckForSerializationFailure(void) offsetof(RWConflictData, inLink)); } + MySerializableXact->prepareSeqNo = ++(PredXact->LastSxactCommitSeqNo); MySerializableXact->flags |= SXACT_FLAG_PREPARED; LWLockRelease(SerializableXactHashLock); @@ -4776,6 +4768,7 @@ predicatelock_twophase_recover(TransactionId xid, uint16 info, sxact->pid = 0; /* a prepared xact hasn't committed yet */ + sxact->prepareSeqNo = RecoverySerCommitSeqNo; sxact->commitSeqNo = InvalidSerCommitSeqNo; sxact->finishedBefore = InvalidTransactionId; diff --git a/src/include/storage/predicate_internals.h b/src/include/storage/predicate_internals.h index 0c90f27..43fcce4 100644 --- a/src/include/storage/predicate_internals.h +++ b/src/include/storage/predicate_internals.h @@ -57,6 +57,7 @@ typedef struct SERIALIZABLEXACT { VirtualTransactionId vxid; /* The executing process always has one of * these. */ + SerCommitSeqNo prepareSeqNo; SerCommitSeqNo commitSeqNo; union /* these values are not both interesting at * the same time */
^ permalink raw reply [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 18:59  Kevin Grittner <Kevin.Grittner@wicourts.gov>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  1 sibling, 1 reply; 26+ messages in thread

From: Kevin Grittner @ 2011-07-07 18:59 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Dan Ports <drkp@csail.mit.edu>; pgsql-hackers

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:
 
> Let's have two sequence numbers for each transaction: prepareSeqNo
> and commitSeqNo. prepareSeqNo is assigned when a transaction is
> prepared (in PreCommit_CheckForSerializableConflicts), and
> commitSeqNo is assigned when it's committed (in
> ReleasePredicateLocks). They are both assigned from one counter,
> LastSxactCommitSeqNo, so that is advanced twice per transaction,
> and prepareSeqNo is always smaller than commitSeqNo for a
> transaction. Modify operations that currently use commitSeqNo to
> use either prepareSeqNo or commitSeqNo, so that we err on the safe
> side.
> 
> That yields a much smaller patch (attached). How does this look to
> you, am I missing anything?
 
Very clever.  I'll need to study this and think about it.  I'll try
to post a response before I go to bed tonight.  Hopefully Dan can
weigh in, too.  (I know he was traveling with limited Internet
access -- not sure about his return date.)
 
-Kevin



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 20:24  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Kevin Grittner <Kevin.Grittner@wicourts.gov>
  0 siblings, 1 reply; 26+ messages in thread

From: Tom Lane @ 2011-07-07 20:24 UTC (permalink / raw)
  To: Kevin Grittner <Kevin.Grittner@wicourts.gov>; +Cc: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; Dan Ports <drkp@csail.mit.edu>; pgsql-hackers

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:
>> That yields a much smaller patch (attached). How does this look to
>> you, am I missing anything?
 
> Very clever.  I'll need to study this and think about it.  I'll try
> to post a response before I go to bed tonight.  Hopefully Dan can
> weigh in, too.  (I know he was traveling with limited Internet
> access -- not sure about his return date.)

Just so everybody's clear --- this isn't making beta3, if it's not going
to get committed today.

			regards, tom lane



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 20:33  Kevin Grittner <Kevin.Grittner@wicourts.gov>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 26+ messages in thread

From: Kevin Grittner @ 2011-07-07 20:33 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Dan Ports <drkp@csail.mit.edu>; Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; pgsql-hackers

Tom Lane <tgl@sss.pgh.pa.us> wrote:
 
> Just so everybody's clear --- this isn't making beta3, if it's not
> going to get committed today.
 
Yeah, Heikki let me know that off-list.  I thought the last mention
on the -hackers list of a cutoff date for that was the 11th.  Did I
misunderstand or was there some later change which didn't get
mentioned on the list?
 
In any event, priorities have been adjusted....
 
Thanks,
 
-Kevin



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 20:35  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  1 sibling, 2 replies; 26+ messages in thread

From: Heikki Linnakangas @ 2011-07-07 20:35 UTC (permalink / raw)
  To: ; +Cc: Kevin Grittner <Kevin.Grittner@wicourts.gov>; pgsql-hackers

On 07.07.2011 21:50, Heikki Linnakangas wrote:
> On 07.07.2011 19:41, Kevin Grittner wrote:
>> Heikki Linnakangas<heikki.linnakangas@enterprisedb.com> wrote:
>>> On 05.07.2011 20:03, Kevin Grittner wrote:
>>>> In reviewing the 2PC changes mentioned in a separate post, both
>>>> Dan and I realized that these were dependent on the assumption
>>>> that SSI's commitSeqNo is assigned in the order in which the
>>>> transactions became visible.
>>>
>>> This comment in the patch actually suggests a stronger
>>> requirement:
>>>
>>>> + * For correct SERIALIZABLE semantics, the commitSeqNo must
>>>> appear to be set
>>>> + * atomically with the work of the transaction becoming visible
>>>> to other
>>>> + * transactions.
>>>
>>> So, is it enough for the commitSeqNos to be set in the order the
>>> transactions become visible to others? I'm assuming 'yes' for now,
>>> as the approach being discussed to assign commitSeqNo in
>>> ProcArrayEndTransaction() without also holding
>>> SerializableXactHashLock is not going to work otherwise, and if I
>>> understood correctly you didn't see any correctness issue with
>>> that. Please shout if I'm missing something.
>>
>> Hmm. The more strict semantics are much easier to reason about and
>> ensure correctness.
>
> True.
>
>> I think that the looser semantics can be made
>> to work, but there needs to be more fussy logic in the SSI code to
>> deal with the fact that we don't know whether the visibility change
>> has occurred. Really what pushed us to the patch using the stricter
>> semantics was how much the discussion of how to cover the edge
>> conditions with the looser semantics made our heads spin. Anything
>> that confusing seems more prone to subtle bugs.
>
> Let's step back and analyse a bit closer what goes wrong with the
> current semantics. The problem is that commitSeqNo is assigned too late,
> sometime after the transaction has become visible to others.
>
> Looking at the comparisons that we do with commitSeqNos, they all have
> conservative direction that we can err to, when in doubt. So here's an
> idea:
>
> Let's have two sequence numbers for each transaction: prepareSeqNo and
> commitSeqNo. prepareSeqNo is assigned when a transaction is prepared (in
> PreCommit_CheckForSerializableConflicts), and commitSeqNo is assigned
> when it's committed (in ReleasePredicateLocks). They are both assigned
> from one counter, LastSxactCommitSeqNo, so that is advanced twice per
> transaction, and prepareSeqNo is always smaller than commitSeqNo for a
> transaction. Modify operations that currently use commitSeqNo to use
> either prepareSeqNo or commitSeqNo, so that we err on the safe side.
>
> That yields a much smaller patch (attached). How does this look to you,
> am I missing anything?

Committed, so that we get this into beta3. We had some quick offlist 
discussion with Keving and Dan about this, Kevin pointed out a few more 
places that needed to use prepareSeqNo instead of commitSeqNo, out of 
which one seemed legitimate to me, and was included in the committed patch.

Kevin and Dan also pointed out that a 2PC transaction can stay in 
prepared state for a long time, and we could optimize that by setting 
prepareSeqNo only at the final COMMIT PREPARED. I objected to that, for 
the reason that it's currently very convenient for testing purposes that 
a transaction prepared with PREPARE TRANSACTION is in the exact same 
state as regular transaction that's going through regular commit processing.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 20:37  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Kevin Grittner <Kevin.Grittner@wicourts.gov>
  0 siblings, 0 replies; 26+ messages in thread

From: Tom Lane @ 2011-07-07 20:37 UTC (permalink / raw)
  To: Kevin Grittner <Kevin.Grittner@wicourts.gov>; +Cc: Dan Ports <drkp@csail.mit.edu>; Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; pgsql-hackers

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Just so everybody's clear --- this isn't making beta3, if it's not
>> going to get committed today.
 
> Yeah, Heikki let me know that off-list.  I thought the last mention
> on the -hackers list of a cutoff date for that was the 11th.  Did I
> misunderstand or was there some later change which didn't get
> mentioned on the list?

The 11th is the announce date.  We normally wrap the Thursday before,
so that packagers (particularly the Windows-installer crew) have some
time to do their thing.

			regards, tom lane



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 20:48  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  1 sibling, 2 replies; 26+ messages in thread

From: Tom Lane @ 2011-07-07 20:48 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Kevin Grittner <Kevin.Grittner@wicourts.gov>; pgsql-hackers

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> Kevin and Dan also pointed out that a 2PC transaction can stay in 
> prepared state for a long time, and we could optimize that by setting 
> prepareSeqNo only at the final COMMIT PREPARED. I objected to that, for 
> the reason that it's currently very convenient for testing purposes that 
> a transaction prepared with PREPARE TRANSACTION is in the exact same 
> state as regular transaction that's going through regular commit processing.

Seems to me there's a more fundamental reason not to do that, which is
that once you've done PREPARE it is no longer legitimate to decide to
roll back the transaction to get out of a "dangerous" structure --- ie,
you have to target one of the other xacts involved instead.  Shouldn't
the assignment of a prepareSeqNo correspond to the point where the xact
is no longer a target for SSI rollback?

			regards, tom lane



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 21:04  Kevin Grittner <Kevin.Grittner@wicourts.gov>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 0 replies; 26+ messages in thread

From: Kevin Grittner @ 2011-07-07 21:04 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers

Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
>> Kevin and Dan also pointed out that a 2PC transaction can stay
>> in prepared state for a long time, and we could optimize that by
>> setting prepareSeqNo only at the final COMMIT PREPARED. I
>> objected to that, for the reason that it's currently very
>> convenient for testing purposes that a transaction prepared with
>> PREPARE TRANSACTION is in the exact same state as regular
>> transaction that's going through regular commit processing.
> 
> Seems to me there's a more fundamental reason not to do that,
> which is that once you've done PREPARE it is no longer legitimate
> to decide to roll back the transaction to get out of a "dangerous"
> structure --- ie, you have to target one of the other xacts
> involved instead. Shouldn't the assignment of a prepareSeqNo
> correspond to the point where the xact is no longer a target for
> SSI rollback?
 
No, it wouldn't be useful at all if we had an accurate commitSeqNo. 
It's being used to bracket the moment of visibility, which with
Heikki's patch now falls between the prepareSeqNo and commitSeqNo. 
The name is perhaps misleading.  It's useful only for determining
what *other* transactions require rollback.
 
In any event, SSI will never cause a transaction to fail after it
has prepared.  This patch doesn't change that, nor would the change
Dan and I suggested.
 
-Kevin



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 21:08  Dan Ports <drkp@csail.mit.edu>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 0 replies; 26+ messages in thread

From: Dan Ports @ 2011-07-07 21:08 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; Kevin Grittner <Kevin.Grittner@wicourts.gov>; pgsql-hackers

On Thu, Jul 07, 2011 at 04:48:55PM -0400, Tom Lane wrote:
> Seems to me there's a more fundamental reason not to do that, which is
> that once you've done PREPARE it is no longer legitimate to decide to
> roll back the transaction to get out of a "dangerous" structure --- ie,
> you have to target one of the other xacts involved instead.  Shouldn't
> the assignment of a prepareSeqNo correspond to the point where the xact
> is no longer a target for SSI rollback?

That part is already accomplished by setting SXACT_FLAG_PREPARED (and
choosing a new victim if we think we want to abort a transaction with
that flag set).

prepareSeqNo is being used as a lower bound on the transaction's commit
sequence number. It's currently set at the same time as the PREPARED
flag, but it doesn't have to be.

Dan

-- 
Dan R. K. Ports              MIT CSAIL                http://drkp.net/



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 21:21  Dan Ports <drkp@csail.mit.edu>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  1 sibling, 2 replies; 26+ messages in thread

From: Dan Ports @ 2011-07-07 21:21 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Kevin Grittner <Kevin.Grittner@wicourts.gov>; pgsql-hackers

We should also apply the attached patch, which corrects a minor issue
with the conditions for flagging transactions that could potentially
make a snapshot unsafe.

There's a small window wherein a transaction is committed but not yet
on the finished list, and we shouldn't flag it as a potential conflict
if so. We can also skip adding a doomed transaction to the list of
possible conflicts because we know it won't commit.

This is not really a related issue, but Kevin and I found it while
looking into this issue, and it was included in the patch we sent out.

Dan

-- 
Dan R. K. Ports              MIT CSAIL                http://drkp.net/

Attachments:

ssi-possibleconflict-test.patch (text/x-diff, 676B) download *** a/src/backend/storage/lmgr/predicate.c --- b/src/backend/storage/lmgr/predicate.c *************** *** 1669,1676 **** RegisterSerializableTransactionInt(Snapshot snapshot) othersxact != NULL; othersxact = NextPredXact(othersxact)) { ! if (!SxactIsOnFinishedList(othersxact) && ! !SxactIsReadOnly(othersxact)) { SetPossibleUnsafeConflict(sxact, othersxact); } --- 1676,1684 ---- othersxact != NULL; othersxact = NextPredXact(othersxact)) { ! if (!SxactIsCommitted(othersxact) ! && !SxactIsDoomed(othersxact) ! && !SxactIsReadOnly(othersxact)) { SetPossibleUnsafeConflict(sxact, othersxact); }
^ permalink raw reply [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 21:24  Kevin Grittner <Kevin.Grittner@wicourts.gov>
  parent: Dan Ports <drkp@csail.mit.edu>
  1 sibling, 0 replies; 26+ messages in thread

From: Kevin Grittner @ 2011-07-07 21:24 UTC (permalink / raw)
  To: Dan Ports <drkp@csail.mit.edu>; Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: pgsql-hackers

Dan Ports <drkp@csail.mit.edu> wrote:
> We should also apply the attached patch, which corrects a minor
> issue with the conditions for flagging transactions that could
> potentially make a snapshot unsafe.
> 
> There's a small window wherein a transaction is committed but not
> yet on the finished list, and we shouldn't flag it as a potential
> conflict if so. We can also skip adding a doomed transaction to
> the list of possible conflicts because we know it won't commit.
> 
> This is not really a related issue, but Kevin and I found it while
> looking into this issue, and it was included in the patch we sent
> out.
 
Agreed -- that was in the SSI atomic commit patch, but probably
should have been submitted separately.  The issue is really
orthogonal -- it's just that we found it while looking at the other
race condition.
 
-Kevin



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 21:33  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Dan Ports <drkp@csail.mit.edu>
  1 sibling, 1 reply; 26+ messages in thread

From: Heikki Linnakangas @ 2011-07-07 21:33 UTC (permalink / raw)
  To: Dan Ports <drkp@csail.mit.edu>; +Cc: Kevin Grittner <Kevin.Grittner@wicourts.gov>; pgsql-hackers

On 08.07.2011 00:21, Dan Ports wrote:
> We should also apply the attached patch, which corrects a minor issue
> with the conditions for flagging transactions that could potentially
> make a snapshot unsafe.
>
> There's a small window wherein a transaction is committed but not yet
> on the finished list, and we shouldn't flag it as a potential conflict
> if so. We can also skip adding a doomed transaction to the list of
> possible conflicts because we know it won't commit.

Hmm, it's now also possible for the transaction to be prepared, and 
already visible to others, but not yet flagged as committed. Shouldn't 
those be included too?

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 21:35  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  0 siblings, 1 reply; 26+ messages in thread

From: Heikki Linnakangas @ 2011-07-07 21:35 UTC (permalink / raw)
  To: ; +Cc: Dan Ports <drkp@csail.mit.edu>; Kevin Grittner <Kevin.Grittner@wicourts.gov>; pgsql-hackers

On 08.07.2011 00:33, Heikki Linnakangas wrote:
> On 08.07.2011 00:21, Dan Ports wrote:
>> We should also apply the attached patch, which corrects a minor issue
>> with the conditions for flagging transactions that could potentially
>> make a snapshot unsafe.
>>
>> There's a small window wherein a transaction is committed but not yet
>> on the finished list, and we shouldn't flag it as a potential conflict
>> if so. We can also skip adding a doomed transaction to the list of
>> possible conflicts because we know it won't commit.
>
> Hmm, it's now also possible for the transaction to be prepared, and
> already visible to others, but not yet flagged as committed. Shouldn't
> those be included too?

Oh, never mind, I read the test backwards. They are included, as the 
patch stands. I'll commit this too..

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com



^ permalink  raw  reply  [nested|flat] 26+ messages in thread

* Re: SSI atomic commit
@ 2011-07-07 21:40  Kevin Grittner <Kevin.Grittner@wicourts.gov>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  0 siblings, 0 replies; 26+ messages in thread

From: Kevin Grittner @ 2011-07-07 21:40 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Dan Ports <drkp@csail.mit.edu>; pgsql-hackers

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:
 
> I'll commit this too..
 
Thanks much for staying up past midnight to get these into beta3!
 
-Kevin



^ permalink  raw  reply  [nested|flat] 26+ messages in thread


end of thread, other threads:[~2011-07-07 21:40 UTC | newest]

Thread overview: 26+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2011-07-05 17:03 SSI atomic commit Kevin Grittner <Kevin.Grittner@wicourts.gov>
2011-07-05 18:01 ` Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2011-07-05 18:15   ` Kevin Grittner <Kevin.Grittner@wicourts.gov>
2011-07-05 19:54     ` Dan Ports <drkp@csail.mit.edu>
2011-07-05 18:16   ` Tom Lane <tgl@sss.pgh.pa.us>
2011-07-05 18:35     ` Kevin Grittner <Kevin.Grittner@wicourts.gov>
2011-07-05 19:17       ` Robert Haas <robertmhaas@gmail.com>
2011-07-05 19:30         ` Tom Lane <tgl@sss.pgh.pa.us>
2011-07-05 19:40           ` Kevin Grittner <Kevin.Grittner@wicourts.gov>
2011-07-05 19:34         ` Kevin Grittner <Kevin.Grittner@wicourts.gov>
2011-07-07 15:45 ` Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2011-07-07 16:41   ` Kevin Grittner <Kevin.Grittner@wicourts.gov>
2011-07-07 18:50     ` Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2011-07-07 18:59       ` Kevin Grittner <Kevin.Grittner@wicourts.gov>
2011-07-07 20:24         ` Tom Lane <tgl@sss.pgh.pa.us>
2011-07-07 20:33           ` Kevin Grittner <Kevin.Grittner@wicourts.gov>
2011-07-07 20:37             ` Tom Lane <tgl@sss.pgh.pa.us>
2011-07-07 20:35       ` Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2011-07-07 20:48         ` Tom Lane <tgl@sss.pgh.pa.us>
2011-07-07 21:04           ` Kevin Grittner <Kevin.Grittner@wicourts.gov>
2011-07-07 21:08           ` Dan Ports <drkp@csail.mit.edu>
2011-07-07 21:21         ` Dan Ports <drkp@csail.mit.edu>
2011-07-07 21:24           ` Kevin Grittner <Kevin.Grittner@wicourts.gov>
2011-07-07 21:33           ` Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2011-07-07 21:35             ` Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2011-07-07 21:40               ` Kevin Grittner <Kevin.Grittner@wicourts.gov>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox