Thread: 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
Chris Cleveland <ccleveland@dieselpoint.com> writes:
> Would it make sense to make GetFreeIndexPage() atomic?
I strongly doubt it. The loss of concurrency would outweigh any
code-simplicity benefits.
regards, tom lane
On Wed, May 4, 2022 at 12:16 PM Chris Cleveland <ccleveland@dieselpoint.com> wrote: > Similar code is repeated in a bunch of places. Each access method has to explicitly write something into a freed page thatidentifies it as ok to use. I wouldn't say that that's what this code is doing, though I do see what you mean. The reason why the ultimate consumer of the free page writes to it is because....well, it wouldn't have asked for a page if it didn't have something to write. Code like GinPageIsRecyclable() is generally concerned with something called recycle safety, typically using something called the drain technique. That's what this code is primarily concerned about. The definition of recycle safety is documented in the nbtree README. > Otherwise, you could have two processes get the same page, then one locks it, writes it, and unlocks it, then the secondone does the same clobbering the first. But you could have that anyway, since the FSM isn't crash safe. It's inherently not completely trustworthy for that reason. Actually, the FSM shouldn't really contain a page that is ! GinPageIsRecyclable() to begin with, but that is still possible for a variety of reasons. All of which boil down to "the current FSM design cannot be totally trusted, so we verify redundantly". -- Peter Geoghegan