Atomic GetFreeIndexPage()?

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