Heikki Linnakangas <heikki.linnakan...@enterprisedb.com> wrote: >> There may be some places this can be checked which haven't yet >> been identified and touched. > > Yeah - in 9.2. No argument here. I'm all for stabilizing and getting the thing out -- I think we've established that performance is good for many workloads as it stands, and that there are workloads where it will never be useful. Chipping away at the gray area, to make it perform well in a few workloads where it currently doesn't (and, of course, *even better* in workloads where it's currently better than the alternatives), seems like future release work to me. There is one issue you raised in this post: http://archives.postgresql.org/message-id/4def3194.6030...@enterprisedb.com Robert questioned whether it should be 9.1 material here: http://archives.postgresql.org/message-id/BANLkTint2i2fHDTdr=Xq3K=yrxegovg...@mail.gmail.com I posted a patch here: http://archives.postgresql.org/message-id/4defb169020000250003e...@gw.wicourts.gov Should I put that patch into a 9.2 CF? There is an unnecessary include of predicate.h in nbtree.c we should delete. That seems safe enough. You questioned whether OldSerXidPagePrecedesLogically was buggy. I will look at that by this weekend at the latest. If it is buggy we obviously should fix that. Are there any other changes you think we should make to handle the odd corner cases in the SLRU for SSI? It did occur to me that we should be safe from actual overwriting of an old entry by the normal transaction wrap-around protections -- the worst that should happen with the current code (I think) is that in extreme cases we may get LOG level messages or accumulate a surprising number of SLRU segment files. That's because SLRU will start to nag about things at one billion transactions, but we need to get all the way to two billion transactions used up while a single serializable transaction remains active before we could overwrite something. It seems like it might be a good idea to apply pgindent formating to the latest SSI changes, to minimize conflict on back-patching any bug fixes. I've attached a patch to do this formatting -- entirely whitespace changes from a pgindent run against selected files. Unless I'm missing something, the only remaining changes needed are for documentation (as mentioned in previous posts). I will work on those after I look at OldSerXidPagePrecedesLogically. -Kevin
*** a/src/backend/access/nbtree/nbtsearch.c --- b/src/backend/access/nbtree/nbtsearch.c *************** *** 849,856 **** _bt_first(IndexScanDesc scan, ScanDirection dir) if (!BufferIsValid(buf)) { /* ! * We only get here if the index is completely empty. ! * Lock relation because nothing finer to lock exists. */ PredicateLockRelation(rel, scan->xs_snapshot); return false; --- 849,856 ---- if (!BufferIsValid(buf)) { /* ! * We only get here if the index is completely empty. Lock relation ! * because nothing finer to lock exists. */ PredicateLockRelation(rel, scan->xs_snapshot); return false; *** a/src/backend/access/transam/twophase_rmgr.c --- b/src/backend/access/transam/twophase_rmgr.c *************** *** 26,32 **** const TwoPhaseCallback twophase_recover_callbacks[TWOPHASE_RM_MAX_ID + 1] = NULL, /* END ID */ lock_twophase_recover, /* Lock */ NULL, /* pgstat */ ! multixact_twophase_recover, /* MultiXact */ predicatelock_twophase_recover /* PredicateLock */ }; --- 26,32 ---- NULL, /* END ID */ lock_twophase_recover, /* Lock */ NULL, /* pgstat */ ! multixact_twophase_recover, /* MultiXact */ predicatelock_twophase_recover /* PredicateLock */ }; *************** *** 44,50 **** const TwoPhaseCallback twophase_postabort_callbacks[TWOPHASE_RM_MAX_ID + 1] = NULL, /* END ID */ lock_twophase_postabort, /* Lock */ pgstat_twophase_postabort, /* pgstat */ ! multixact_twophase_postabort, /* MultiXact */ NULL /* PredicateLock */ }; --- 44,50 ---- NULL, /* END ID */ lock_twophase_postabort, /* Lock */ pgstat_twophase_postabort, /* pgstat */ ! multixact_twophase_postabort, /* MultiXact */ NULL /* PredicateLock */ }; *** a/src/backend/storage/lmgr/predicate.c --- b/src/backend/storage/lmgr/predicate.c *************** *** 483,492 **** SerializationNeededForRead(Relation relation, Snapshot snapshot) * MySerializableXact, so that subsequent calls to this function can exit * quickly. * ! * A transaction is flagged as RO_SAFE if all concurrent R/W ! * transactions commit without having conflicts out to an earlier ! * snapshot, thus ensuring that no conflicts are possible for this ! * transaction. */ if (SxactIsROSafe(MySerializableXact)) { --- 483,491 ---- * MySerializableXact, so that subsequent calls to this function can exit * quickly. * ! * A transaction is flagged as RO_SAFE if all concurrent R/W transactions ! * commit without having conflicts out to an earlier snapshot, thus ! * ensuring that no conflicts are possible for this transaction. */ if (SxactIsROSafe(MySerializableXact)) { *************** *** 498,504 **** SerializationNeededForRead(Relation relation, Snapshot snapshot) if (!PredicateLockingNeededForRelation(relation)) return false; ! return true; /* no excuse to skip predicate locking */ } /* --- 497,503 ---- if (!PredicateLockingNeededForRelation(relation)) return false; ! return true; /* no excuse to skip predicate locking */ } /* *************** *** 516,522 **** SerializationNeededForWrite(Relation relation) if (!PredicateLockingNeededForRelation(relation)) return false; ! return true; /* no excuse to skip predicate locking */ } --- 515,521 ---- if (!PredicateLockingNeededForRelation(relation)) return false; ! return true; /* no excuse to skip predicate locking */ } *** a/src/include/storage/predicate.h --- b/src/include/storage/predicate.h *************** *** 54,60 **** extern void ReleasePredicateLocks(const bool isCommit); /* conflict detection (may also trigger rollback) */ extern void CheckForSerializableConflictOut(const bool valid, const Relation relation, const HeapTuple tuple, ! const Buffer buffer, const Snapshot snapshot); extern void CheckForSerializableConflictIn(const Relation relation, const HeapTuple tuple, const Buffer buffer); extern void CheckTableForSerializableConflictIn(const Relation relation); --- 54,60 ---- /* conflict detection (may also trigger rollback) */ extern void CheckForSerializableConflictOut(const bool valid, const Relation relation, const HeapTuple tuple, ! const Buffer buffer, const Snapshot snapshot); extern void CheckForSerializableConflictIn(const Relation relation, const HeapTuple tuple, const Buffer buffer); extern void CheckTableForSerializableConflictIn(const Relation relation); *** a/src/include/storage/predicate_internals.h --- b/src/include/storage/predicate_internals.h *************** *** 90,98 **** 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_DOOMED 0x00000004 /* 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 --- 90,98 ---- 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_DOOMED 0x00000004 /* 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 *************** *** 132,139 **** typedef struct PredXactListData /* * These global variables are maintained when registering and cleaning up * serializable transactions. They must be global across all backends, ! * but are not needed outside the predicate.c source file. Protected ! * by SerializableXactHashLock. */ TransactionId SxactGlobalXmin; /* global xmin for active serializable * transactions */ --- 132,139 ---- /* * These global variables are maintained when registering and cleaning up * serializable transactions. They must be global across all backends, ! * but are not needed outside the predicate.c source file. Protected by ! * SerializableXactHashLock. */ TransactionId SxactGlobalXmin; /* global xmin for active serializable * transactions */
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers