[prev in list] [next in list] [prev in thread] [next in thread]
List: postgresql-general
Subject: Re: [HACKERS] Speed up Clog Access by increasing CLOG buffers
From: Amit Kapila <amit.kapila16 () gmail ! com>
Date: 2016-12-31 3:43:05
Message-ID: CAA4eK1J+67edo_Wnrfx8oJ+rWM_BAr+v6JqvQjKPdLOxR=0d5g () mail ! gmail ! com
[Download RAW message or body]
On Thu, Dec 29, 2016 at 10:41 AM, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> I have done one more pass of the review today. I have few comments.
>
> + if (nextidx != INVALID_PGPROCNO)
> + {
> + /* Sleep until the leader updates our XID status. */
> + for (;;)
> + {
> + /* acts as a read barrier */
> + PGSemaphoreLock(&proc->sem);
> + if (!proc->clogGroupMember)
> + break;
> + extraWaits++;
> + }
> +
> + Assert(pg_atomic_read_u32(&proc->clogGroupNext) == INVALID_PGPROCNO);
> +
> + /* Fix semaphore count for any absorbed wakeups */
> + while (extraWaits-- > 0)
> + PGSemaphoreUnlock(&proc->sem);
> + return true;
> + }
>
> 1. extraWaits is used only locally in this block so I guess we can
> declare inside this block only.
>
Agreed and changed accordingly.
> 2. It seems that we have missed one unlock in case of absorbed
> wakeups. You have initialised extraWaits with -1 and if there is one
> extra wake up then extraWaits will become 0 (it means we have made one
> extra call to PGSemaphoreLock and it's our responsibility to fix it as
> the leader will Unlock only once). But it appear in such case we will
> not make any call to PGSemaphoreUnlock.
>
Good catch! I have fixed it by initialising extraWaits to 0. This
same issue exists from Group clear xid for which I will send a patch
separately.
Apart from above, the patch needs to be adjusted for commit be7b2848
which has changed the definition of PGSemaphore.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
["group_update_clog_v10.patch" (application/octet-stream)]
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 2634476..3dd5b97 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -35,11 +35,13 @@
#include "access/clog.h"
#include "access/slru.h"
#include "access/transam.h"
+#include "access/twophase.h"
#include "access/xlog.h"
#include "access/xloginsert.h"
#include "access/xlogutils.h"
#include "miscadmin.h"
#include "pg_trace.h"
+#include "storage/proc.h"
/*
* Defines for CLOG page sizes. A page is the same BLCKSZ as is used
@@ -86,11 +88,17 @@ static void WriteZeroPageXlogRec(int pageno);
static void WriteTruncateXlogRec(int pageno);
static void TransactionIdSetPageStatus(TransactionId xid, int nsubxids,
TransactionId *subxids, XidStatus status,
- XLogRecPtr lsn, int pageno);
+ XLogRecPtr lsn, int pageno,
+ bool all_xact_same_page);
static void TransactionIdSetStatusBit(TransactionId xid, XidStatus status,
XLogRecPtr lsn, int slotno);
static void set_status_by_pages(int nsubxids, TransactionId *subxids,
XidStatus status, XLogRecPtr lsn);
+static bool TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
+ XLogRecPtr lsn, int pageno);
+static void TransactionIdSetPageStatusInternal(TransactionId xid, int nsubxids,
+ TransactionId *subxids, XidStatus status,
+ XLogRecPtr lsn, int pageno);
/*
@@ -173,7 +181,7 @@ TransactionIdSetTreeStatus(TransactionId xid, int nsubxids,
* Set the parent and all subtransactions in a single call
*/
TransactionIdSetPageStatus(xid, nsubxids, subxids, status, lsn,
- pageno);
+ pageno, true);
}
else
{
@@ -200,7 +208,7 @@ TransactionIdSetTreeStatus(TransactionId xid, int nsubxids,
*/
pageno = TransactionIdToPage(xid);
TransactionIdSetPageStatus(xid, nsubxids_on_first_page, subxids, status,
- lsn, pageno);
+ lsn, pageno, false);
/*
* Now work through the rest of the subxids one clog page at a time,
@@ -238,7 +246,7 @@ set_status_by_pages(int nsubxids, TransactionId *subxids,
TransactionIdSetPageStatus(InvalidTransactionId,
num_on_page, subxids + offset,
- status, lsn, pageno);
+ status, lsn, pageno, false);
offset = i;
pageno = TransactionIdToPage(subxids[offset]);
}
@@ -248,12 +256,78 @@ set_status_by_pages(int nsubxids, TransactionId *subxids,
* Record the final state of transaction entries in the commit log for
* all entries on a single page. Atomic only on this page.
*
+ * Group the status update for transactions. This improves the efficiency
+ * of the transaction status update by reducing the number of lock
+ * acquisitions required for it. We need to populate the transaction status
+ * related information in shared memory, so that group leader can read that
+ * information and perform the transaction status update request.
+ *
+ * The group status update optimization doesn't apply in below cases:
+ * a) For the overflowed sub-transactions, as that would need a big chunk of
+ * shared memory to populate each sub-transaction status information.
+ * b) For the transactions where status of transaction and all its child
+ * sub-transactions doesn't belong to same page. To apply the optimization
+ * for this case, we need to map sub-transactions in proc's XidCache based
+ * on pageno for which each time a group leader needs to set the transaction
+ * status. Now that can lead to some performance penalty as that needs to
+ * be done after acquiring CLogControlLock, so let's leave that case for now.
+ * c) For the prepared transactions, as the dummy proc associated with such
+ * transactions doesn't have a semaphore associated with it and the same is
+ * required for group status update. We choose not to create a semaphore
+ * for dummy procs for this purpose as the advantage of using this
+ * optimization for prepared transactions is not clear.
+ *
* Otherwise API is same as TransactionIdSetTreeStatus()
*/
static void
TransactionIdSetPageStatus(TransactionId xid, int nsubxids,
TransactionId *subxids, XidStatus status,
- XLogRecPtr lsn, int pageno)
+ XLogRecPtr lsn, int pageno,
+ bool all_xact_same_page)
+{
+ if (all_xact_same_page &&
+ nsubxids < PGPROC_MAX_CACHED_SUBXIDS &&
+ !IsGXactActive())
+ {
+ /*
+ * If we can immediately acquire CLogControlLock, we update the status
+ * of our own XID and release the lock. If not, use group XID status
+ * update to improve efficiency and if still not able to update, then
+ * acquire CLogControlLock and update it.
+ */
+ if (LWLockConditionalAcquire(CLogControlLock, LW_EXCLUSIVE))
+ {
+ TransactionIdSetPageStatusInternal(xid, nsubxids, subxids, status, lsn, pageno);
+ LWLockRelease(CLogControlLock);
+ }
+ else if (!TransactionGroupUpdateXidStatus(xid, status, lsn, pageno))
+ {
+ LWLockAcquire(CLogControlLock, LW_EXCLUSIVE);
+
+ TransactionIdSetPageStatusInternal(xid, nsubxids, subxids, status, lsn, pageno);
+
+ LWLockRelease(CLogControlLock);
+ }
+ }
+ else
+ {
+ LWLockAcquire(CLogControlLock, LW_EXCLUSIVE);
+
+ TransactionIdSetPageStatusInternal(xid, nsubxids, subxids, status, lsn, pageno);
+
+ LWLockRelease(CLogControlLock);
+ }
+}
+
+/*
+ * Record the final state of transaction entry in the commit log
+ *
+ * We don't do any locking here; caller must handle that.
+ */
+static void
+TransactionIdSetPageStatusInternal(TransactionId xid, int nsubxids,
+ TransactionId *subxids, XidStatus status,
+ XLogRecPtr lsn, int pageno)
{
int slotno;
int i;
@@ -262,8 +336,6 @@ TransactionIdSetPageStatus(TransactionId xid, int nsubxids,
status == TRANSACTION_STATUS_ABORTED ||
(status == TRANSACTION_STATUS_SUB_COMMITTED && !TransactionIdIsValid(xid)));
- LWLockAcquire(CLogControlLock, LW_EXCLUSIVE);
-
/*
* If we're doing an async commit (ie, lsn is valid), then we must wait
* for any active write on the page slot to complete. Otherwise our
@@ -310,8 +382,172 @@ TransactionIdSetPageStatus(TransactionId xid, int nsubxids,
}
ClogCtl->shared->page_dirty[slotno] = true;
+}
+/*
+ * When we cannot immediately acquire CLogControlLock in exclusive mode at
+ * commit time, add ourselves to a list of processes that need their XIDs
+ * status update. The first process to add itself to the list will acquire
+ * CLogControlLock in exclusive mode and perform TransactionIdSetPageStatusInternal
+ * on behalf of all group members. This avoids a great deal of contention
+ * around CLogControlLock when many processes are trying to commit at once,
+ * since the lock need not be repeatedly handed off from one committing
+ * process to the next.
+ *
+ * Returns true, if transaction status is updated in clog page, else return
+ * false.
+ */
+static bool
+TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
+ XLogRecPtr lsn, int pageno)
+{
+ volatile PROC_HDR *procglobal = ProcGlobal;
+ PGPROC *proc = MyProc;
+ uint32 nextidx;
+ uint32 wakeidx;
+
+ /* We should definitely have an XID whose status needs to be updated. */
+ Assert(TransactionIdIsValid(xid));
+
+ /*
+ * Add ourselves to the list of processes needing a group XID status
+ * update.
+ */
+ proc->clogGroupMember = true;
+ proc->clogGroupMemberXid = xid;
+ proc->clogGroupMemberXidStatus = status;
+ proc->clogGroupMemberPage = pageno;
+ proc->clogGroupMemberLsn = lsn;
+
+ nextidx = pg_atomic_read_u32(&procglobal->clogGroupFirst);
+
+ while (true)
+ {
+ /*
+ * Add the proc to list, if the clog page where we need to update the
+ * current transaction status is same as group leader's clog page.
+ *
+ * There is a race condition here, which is that after doing the below
+ * check and before adding this proc's clog update to a group, the
+ * group leader might have already finished the group update for this
+ * page and becomes group leader of another group. This will lead to a
+ * situation where a single group can have different clog page
+ * updates. This is harmless and the chances of such a race condition
+ * are less and even if it happens, the only downside is that it could
+ * lead to serial access of clog pages from disk if those pages are
+ * not in memory. Tests doesn't indicate any performance hit due to
+ * different clog page updates in same group. However in future, if
+ * we want to improve the situation, then we can detect the non-group
+ * leader transactions that tries to update the different CLOG page
+ * after acquiring CLogControlLock and mark these transactions such
+ * that after waking they need to perform CLOG update via normal path.
+ */
+ if (nextidx != INVALID_PGPROCNO &&
+ ProcGlobal->allProcs[nextidx].clogGroupMemberPage != proc->clogGroupMemberPage)
+ {
+ proc->clogGroupMember = false;
+ return false;
+ }
+
+ pg_atomic_write_u32(&proc->clogGroupNext, nextidx);
+
+ if (pg_atomic_compare_exchange_u32(&procglobal->clogGroupFirst,
+ &nextidx,
+ (uint32) proc->pgprocno))
+ break;
+ }
+
+ /*
+ * If the list was not empty, the leader will update the status of our
+ * XID. It is impossible to have followers without a leader because the
+ * first process that has added itself to the list will always have
+ * nextidx as INVALID_PGPROCNO.
+ */
+ if (nextidx != INVALID_PGPROCNO)
+ {
+ int extraWaits = 0;
+
+ /* Sleep until the leader updates our XID status. */
+ for (;;)
+ {
+ /* acts as a read barrier */
+ PGSemaphoreLock(proc->sem);
+ if (!proc->clogGroupMember)
+ break;
+ extraWaits++;
+ }
+
+ Assert(pg_atomic_read_u32(&proc->clogGroupNext) == INVALID_PGPROCNO);
+
+ /* Fix semaphore count for any absorbed wakeups */
+ while (extraWaits-- > 0)
+ PGSemaphoreUnlock(proc->sem);
+ return true;
+ }
+
+ /* We are the leader. Acquire the lock on behalf of everyone. */
+ LWLockAcquire(CLogControlLock, LW_EXCLUSIVE);
+
+ /*
+ * Now that we've got the lock, clear the list of processes waiting for
+ * group XID status update, saving a pointer to the head of the list.
+ * Trying to pop elements one at a time could lead to an ABA problem.
+ */
+ nextidx = pg_atomic_exchange_u32(&procglobal->clogGroupFirst, INVALID_PGPROCNO);
+
+ /* Remember head of list so we can perform wakeups after dropping lock. */
+ wakeidx = nextidx;
+
+ /* Walk the list and update the status of all XIDs. */
+ while (nextidx != INVALID_PGPROCNO)
+ {
+ PGPROC *proc = &ProcGlobal->allProcs[nextidx];
+ PGXACT *pgxact = &ProcGlobal->allPgXact[nextidx];
+
+ /*
+ * Overflowed transactions should not use group XID status update
+ * mechanism.
+ */
+ Assert(!pgxact->overflowed);
+
+ TransactionIdSetPageStatusInternal(proc->clogGroupMemberXid,
+ pgxact->nxids,
+ proc->subxids.xids,
+ proc->clogGroupMemberXidStatus,
+ proc->clogGroupMemberLsn,
+ proc->clogGroupMemberPage);
+
+ /* Move to next proc in list. */
+ nextidx = pg_atomic_read_u32(&proc->clogGroupNext);
+ }
+
+ /* We're done with the lock now. */
LWLockRelease(CLogControlLock);
+
+ /*
+ * Now that we've released the lock, go back and wake everybody up. We
+ * don't do this under the lock so as to keep lock hold times to a
+ * minimum. The system calls we need to perform to wake other processes
+ * up are probably slower and can cause performance slowdown if done under
+ * lock.
+ */
+ while (wakeidx != INVALID_PGPROCNO)
+ {
+ PGPROC *proc = &ProcGlobal->allProcs[wakeidx];
+
+ wakeidx = pg_atomic_read_u32(&proc->clogGroupNext);
+ pg_atomic_write_u32(&proc->clogGroupNext, INVALID_PGPROCNO);
+
+ /* ensure all previous writes are visible before follower continues. */
+ pg_write_barrier();
+
+ proc->clogGroupMember = false;
+
+ if (proc != MyProc)
+ PGSemaphoreUnlock(proc->sem);
+ }
+
+ return true;
}
/*
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 5415604..28d7d72 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -177,7 +177,7 @@ static TwoPhaseStateData *TwoPhaseState;
/*
* Global transaction entry currently locked by us, if any.
*/
-static GlobalTransaction MyLockedGxact = NULL;
+GlobalTransaction MyLockedGxact = NULL;
static bool twophaseExitRegistered = false;
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index e9555f2..95d224b 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -186,6 +186,7 @@ InitProcGlobal(void)
ProcGlobal->walwriterLatch = NULL;
ProcGlobal->checkpointerLatch = NULL;
pg_atomic_init_u32(&ProcGlobal->procArrayGroupFirst, INVALID_PGPROCNO);
+ pg_atomic_init_u32(&ProcGlobal->clogGroupFirst, INVALID_PGPROCNO);
/*
* Create and initialize all the PGPROC structures we'll need. There are
@@ -407,6 +408,14 @@ InitProcess(void)
/* Initialize wait event information. */
MyProc->wait_event_info = 0;
+ /* Initialize fields for group transaction status update. */
+ MyProc->clogGroupMember = false;
+ MyProc->clogGroupMemberXid = InvalidTransactionId;
+ MyProc->clogGroupMemberXidStatus = TRANSACTION_STATUS_IN_PROGRESS;
+ MyProc->clogGroupMemberPage = -1;
+ MyProc->clogGroupMemberLsn = InvalidXLogRecPtr;
+ pg_atomic_init_u32(&MyProc->clogGroupNext, INVALID_PGPROCNO);
+
/*
* Acquire ownership of the PGPROC's latch, so that we can use WaitLatch
* on it. That allows us to repoint the process latch, which so far
diff --git a/src/include/access/twophase.h b/src/include/access/twophase.h
index b7ce0c6..31d7666 100644
--- a/src/include/access/twophase.h
+++ b/src/include/access/twophase.h
@@ -24,6 +24,8 @@
*/
typedef struct GlobalTransactionData *GlobalTransaction;
+extern GlobalTransaction MyLockedGxact;
+
/* GUC variable */
extern int max_prepared_xacts;
@@ -36,6 +38,17 @@ extern void PostPrepare_Twophase(void);
extern PGPROC *TwoPhaseGetDummyProc(TransactionId xid);
extern BackendId TwoPhaseGetDummyBackendId(TransactionId xid);
+/*
+ * IsGXactActive
+ * Return true if there is a Global transaction entry currently
+ * locked by us.
+ */
+static inline bool
+IsGXactActive(void)
+{
+ return MyLockedGxact ? true : false;
+}
+
extern GlobalTransaction MarkAsPreparing(TransactionId xid, const char *gid,
TimestampTz prepared_at,
Oid owner, Oid databaseid);
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 0344f42..8611823 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -14,6 +14,7 @@
#ifndef _PROC_H_
#define _PROC_H_
+#include "access/clog.h"
#include "access/xlogdefs.h"
#include "lib/ilist.h"
#include "storage/latch.h"
@@ -159,6 +160,17 @@ struct PGPROC
uint32 wait_event_info; /* proc's wait information */
+ /* Support for group transaction status update. */
+ bool clogGroupMember; /* true, if member of clog group */
+ pg_atomic_uint32 clogGroupNext; /* next clog group member */
+ TransactionId clogGroupMemberXid; /* transaction id of clog group member */
+ XidStatus clogGroupMemberXidStatus; /* transaction status of clog
+ * group member */
+ int clogGroupMemberPage; /* clog page corresponding to
+ * transaction id of clog group member */
+ XLogRecPtr clogGroupMemberLsn; /* WAL location of commit record for
+ * clog group member */
+
/* Per-backend LWLock. Protects fields below (but not group fields). */
LWLock backendLock;
@@ -230,6 +242,8 @@ typedef struct PROC_HDR
PGPROC *bgworkerFreeProcs;
/* First pgproc waiting for group XID clear */
pg_atomic_uint32 procArrayGroupFirst;
+ /* First pgproc waiting for group transaction status update */
+ pg_atomic_uint32 clogGroupFirst;
/* WALWriter process's latch */
Latch *walwriterLatch;
/* Checkpointer process's latch */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic