Would it make sense to make GetFreeIndexPage() atomic?
Internally, GetFreeIndexPage() calls GetPageWithFreeSpace() and then RecordUsedIndexPage() in two separate operations. It's possible for two different processes to get the same free page at the same time.
To guard against this, there are several index access methods that do something like this:
/* First, try to get a page from FSM */
for (;;)
{
BlockNumber blkno = GetFreeIndexPage(index);
if (blkno == InvalidBlockNumber)
break;
buffer = ReadBuffer(index, blkno);
/*
* We have to guard against the possibility that someone else already
* recycled this page; the buffer may be locked if so.
*/
if (ConditionalLockBuffer(buffer))
{
if (GinPageIsRecyclable(BufferGetPage(buffer)))
return buffer; /* OK to use */
LockBuffer(buffer, GIN_UNLOCK);
}
/* Can't use it, so release buffer and try again */
ReleaseBuffer(buffer);
}
Similar code is repeated in a bunch of places. Each access method has to explicitly write something into a freed page that identifies it as ok to use. Otherwise, you could have two processes get the same page, then one locks it, writes it, and unlocks it, then the second one does the same clobbering the first.
All this logic goes away if GetFreeIndexPage() is atomic, such that finding and marking a page as not-free always happens in one go. No longer would the index pages themselves need to be modified to identify them as available.
I'm thinking of surrounding the code that calls GetFreeIndexPage() with a lightweight lock, although I wonder if that would harm performance. Perhaps there is a more performant way to do this deep down in the FSM code.
Thoughts?
--
Chris Cleveland
312-339-2677 mobile
