Re: MarkBufferDirty Assert held LW_EXCLUSIVE lock fail when ginFinishSplit

On 22/01/2024 18:21, feichanghong wrote:
> 
>> From a performance point of view, this doesn't matter. Incomplete 
>> split are extremely rare. For convenience, though, I added a new 
>> function specifically for handling these "leftover" incomplete splits 
>> as opposed to finishing a split that you just made, which performs the 
>> lock-upgrade. See attached. I think that helps with readability, and 
>> makes it less likely that we'll forget the lock-upgrade in the future 
>> if the insertion code is refactored.
> I think that the lock-upgrade in the ginFinishOldSplit function is unsafe
> because it violates the requirement of the ginStepRight function that
> "The next page is locked first, before releasing the current page.”

Good point.

I started to work on a more invasive patch that would move the 
ginFinishOldSplit() calls to ginTraverseLock() and ginStepRight(), doing 
the interlocking correctly. That makes life easier for the callers; they 
don't need to deal with incomplete-splits anymore.

But then I read the Page deletion section in the README and understood 
that my earlier patch is safe, after all. The lock-coupling in 
ginStepRight() is only needed for searches, not for inserts. There is 
another mechanism that prevents concurrent page deletions during an 
insert: VACUUM holds a cleanup-lock on the root page.

Does that make sense, or am I missing something? Here's the same patch 
as before, with some extra comments to explain why it's safe.

-- 
Heikki Linnakangas
Neon (https://neon.tech)