From e09147a91d20e3c86da26e311518f92aea193bc1 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi Date: Tue, 1 Nov 2016 18:28:58 +0900 Subject: [PATCH] Alternative Type 1. Maintenance progressAt in SpinLock(Insert->insertpos_lck) section in ReserveXLogInsertLocation. Even if additinal code into the section does no harm, GetProgressRecPtr also needs to take the same lock. --- src/backend/access/transam/xlog.c | 56 ++++++++++----------------------------- 1 file changed, 14 insertions(+), 42 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 1eff059..0703e5b 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -466,7 +466,6 @@ typedef struct { LWLock lock; XLogRecPtr insertingAt; - XLogRecPtr progressAt; } WALInsertLock; /* @@ -672,9 +671,13 @@ typedef struct XLogCtlData */ XLogRecPtr lastFpwDisableRecPtr; + uint64 progressAt; /* This is not locked by info_lck but it is + * here for just convenience. */ + slock_t info_lck; /* locks shared variables shown above */ } XLogCtlData; + static XLogCtlData *XLogCtl = NULL; /* a private copy of XLogCtl->Insert.WALInsertLocks, for convenience */ @@ -1021,25 +1024,6 @@ XLogInsertRecord(XLogRecData *rdata, inserted = true; } - /* - * Update the progress LSN positions. At least one WAL insertion lock - * is already taken appropriately before doing that, and it is just more - * simple to do that here where WAL record data and type is at hand. - * The progress is set at the start position of the record tracked that - * is being added, making easier checkpoint progress tracking as the - * control file already saves the start LSN position of the last - * checkpoint run. If an exclusive lock is taken for WAL insertion, - * there is actually no need to update all the progression fields, so - * just do it on the first one. - */ - if ((flags & XLOG_NO_PROGRESS) == 0) - { - if (holdingAllLocks) - WALInsertLocks[0].l.progressAt = StartPos; - else - WALInsertLocks[MyLockNo].l.progressAt = StartPos; - } - if (inserted) { /* @@ -1195,6 +1179,7 @@ static void ReserveXLogInsertLocation(int size, XLogRecPtr *StartPos, XLogRecPtr *EndPos, XLogRecPtr *PrevPtr) { + volatile XLogCtlData *xlogctl = XLogCtl; XLogCtlInsert *Insert = &XLogCtl->Insert; uint64 startbytepos; uint64 endbytepos; @@ -1222,6 +1207,8 @@ ReserveXLogInsertLocation(int size, XLogRecPtr *StartPos, XLogRecPtr *EndPos, prevbytepos = Insert->PrevBytePos; Insert->CurrBytePos = endbytepos; Insert->PrevBytePos = startbytepos; + if (xlogctl->progressAt < startbytepos) + xlogctl->progressAt = startbytepos; SpinLockRelease(&Insert->insertpos_lck); @@ -4763,7 +4750,6 @@ XLOGShmemInit(void) { LWLockInitialize(&WALInsertLocks[i].l.lock, LWTRANCHE_WAL_INSERT); WALInsertLocks[i].l.insertingAt = InvalidXLogRecPtr; - WALInsertLocks[i].l.progressAt = InvalidXLogRecPtr; } /* @@ -8050,29 +8036,15 @@ GetFlushRecPtr(void) XLogRecPtr GetProgressRecPtr(void) { - XLogRecPtr res = InvalidXLogRecPtr; - int i; + volatile XLogCtlData *xlogctl = XLogCtl; + uint32 bret; - /* - * Look at the latest LSN position referring to the activity done by - * WAL insertion. An exclusive lock is taken because currently the - * locking logic for WAL insertion only expects such a level of locking. - * Taking a lock is as well necessary to prevent potential torn reads - * on some platforms. - */ - for (i = 0; i < NUM_XLOGINSERT_LOCKS; i++) - { - XLogRecPtr progress_lsn; - - LWLockAcquire(&WALInsertLocks[i].l.lock, LW_EXCLUSIVE); - progress_lsn = WALInsertLocks[i].l.progressAt; - LWLockRelease(&WALInsertLocks[i].l.lock); - - if (res < progress_lsn) - res = progress_lsn; - } + /* XXXX: Taking insertpos_lck is too bad but it's necessary :( */ + SpinLockAcquire(&xlogctl->Insert.insertpos_lck); + bret = xlogctl->progressAt; + SpinLockRelease(&xlogctl->Insert.insertpos_lck); - return res; + return XLogBytePosToRecPtr(bret); } /* -- 2.9.2