Commit dc02a481 authored by Tom Lane's avatar Tom Lane

Fix a race condition that I introduced into sinvaladt.c during the recent

rewrite.  When called from SIInsertDataEntries, SICleanupQueue releases
the write lock if it has to issue a kill() to signal some laggard backend.
That still seems like a good idea --- but it's possible that by the time
we get the lock back, there are no longer enough free message slots to
satisfy SIInsertDataEntries' requirement.  Must recheck, and repeat the
whole SICleanupQueue process if not.  Noted while reading code.
parent a4775a80
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
* *
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/storage/ipc/sinvaladt.c,v 1.73 2008/07/01 02:09:34 tgl Exp $ * $PostgreSQL: pgsql/src/backend/storage/ipc/sinvaladt.c,v 1.74 2008/07/18 14:45:48 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
...@@ -296,8 +296,6 @@ SharedInvalBackendInit(void) ...@@ -296,8 +296,6 @@ SharedInvalBackendInit(void)
MyBackendId = (stateP - &segP->procState[0]) + 1; MyBackendId = (stateP - &segP->procState[0]) + 1;
elog(DEBUG4, "my backend id is %d", MyBackendId);
/* Advertise assigned backend ID in MyProc */ /* Advertise assigned backend ID in MyProc */
MyProc->backendId = MyBackendId; MyProc->backendId = MyBackendId;
...@@ -314,6 +312,8 @@ SharedInvalBackendInit(void) ...@@ -314,6 +312,8 @@ SharedInvalBackendInit(void)
/* register exit routine to mark my entry inactive at exit */ /* register exit routine to mark my entry inactive at exit */
on_shmem_exit(CleanupInvalidationState, PointerGetDatum(segP)); on_shmem_exit(CleanupInvalidationState, PointerGetDatum(segP));
elog(DEBUG4, "my backend id is %d", MyBackendId);
} }
/* /*
...@@ -416,16 +416,18 @@ SIInsertDataEntries(const SharedInvalidationMessage *data, int n) ...@@ -416,16 +416,18 @@ SIInsertDataEntries(const SharedInvalidationMessage *data, int n)
* If the buffer is full, we *must* acquire some space. Clean the * If the buffer is full, we *must* acquire some space. Clean the
* queue and reset anyone who is preventing space from being freed. * queue and reset anyone who is preventing space from being freed.
* Otherwise, clean the queue only when it's exceeded the next * Otherwise, clean the queue only when it's exceeded the next
* fullness threshold. * fullness threshold. We have to loop and recheck the buffer state
* after any call of SICleanupQueue.
*/ */
numMsgs = segP->maxMsgNum - segP->minMsgNum; for (;;)
if (numMsgs + nthistime > MAXNUMMESSAGES)
{ {
SICleanupQueue(true, nthistime); numMsgs = segP->maxMsgNum - segP->minMsgNum;
Assert((segP->maxMsgNum - segP->minMsgNum + nthistime) <= MAXNUMMESSAGES); if (numMsgs + nthistime > MAXNUMMESSAGES ||
numMsgs >= segP->nextThreshold)
SICleanupQueue(true, nthistime);
else
break;
} }
else if (numMsgs >= segP->nextThreshold)
SICleanupQueue(true, 0);
/* /*
* Insert new message(s) into proper slot of circular buffer * Insert new message(s) into proper slot of circular buffer
...@@ -546,12 +548,16 @@ SIGetDataEntries(SharedInvalidationMessage *data, int datasize) ...@@ -546,12 +548,16 @@ SIGetDataEntries(SharedInvalidationMessage *data, int datasize)
* Remove messages that have been consumed by all active backends * Remove messages that have been consumed by all active backends
* *
* callerHasWriteLock is TRUE if caller is holding SInvalWriteLock. * callerHasWriteLock is TRUE if caller is holding SInvalWriteLock.
* minFree is the minimum number of free message slots required at completion. * minFree is the minimum number of message slots to make free.
* *
* Possible side effects of this routine include marking one or more * Possible side effects of this routine include marking one or more
* backends as "reset" in the array, and sending a catchup interrupt (SIGUSR1) * backends as "reset" in the array, and sending a catchup interrupt (SIGUSR1)
* to some backend that seems to be getting too far behind. We signal at * to some backend that seems to be getting too far behind. We signal at
* most one backend at a time, for reasons explained at the top of the file. * most one backend at a time, for reasons explained at the top of the file.
*
* Caution: because we transiently release write lock when we have to signal
* some other backend, it is NOT guaranteed that there are still minFree
* free message slots at exit. Caller must recheck and perhaps retry.
*/ */
void void
SICleanupQueue(bool callerHasWriteLock, int minFree) SICleanupQueue(bool callerHasWriteLock, int minFree)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment