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

Reply via email to