From: Heikki Linnakangas <hlinnaka@iki.fi>
To: feichanghong <feichanghong@qq.com>
To: pgsql-bugs <pgsql-bugs@lists.postgresql.org>
Subject: Re: MarkBufferDirty Assert held LW_EXCLUSIVE lock fail when ginFinishSplit
Date: Mon, 22 Jan 2024 11:42:20 +0200
Message-ID: <053bc845-3cb5-445e-8ea4-00f6c678ee25@iki.fi> (raw)
In-Reply-To: <tencent_A3CE810F59132D8E230475A5F0F7A08C8307@qq.com>
References: <tencent_A3CE810F59132D8E230475A5F0F7A08C8307@qq.com>
On 22/01/2024 09:59, feichanghong wrote:
> In the function ginFindLeafPage, if we encounter a page marked with
> GIN_INCOMPLETE_SPLIT, we call ginFinishSplit to finish the incomplete
> split and
> remove the GIN_INCOMPLETE_SPLIT flag from that page. ginFinishSplit requires
> that "On entry, stack->buffer is exclusively locked," as explained in its
> comments.
>
> The function ginFindLeafPage determines the type of lock held by
> ginTraverseLock:
> leaf pages hold GIN_EXCLUSIVE, and internal page hold GIN_SHARE. If
> ginFindLeafPage
> traverses to an internal page with an incomplete split, it will only hold a
> GIN_SHARE lock, which does not meet the requirements of ginFinishSplit. If a
> crash occurs when an internal page is split, but no downlink is inserted
> in the
> parent page, this problem may occur.
>
> I injected the following code into the ginbtree.c file to trigger a crash
> during the splitting of an internal page:
> ```
> --- a/src/backend/access/gin/ginbtree.c
> +++ b/src/backend/access/gin/ginbtree.c
> @@ -621,6 +621,12 @@ ginPlaceToPage(GinBtree btree, GinBtreeStack *stack,
> PageSetLSN(BufferGetPage(lbuffer), recptr);
> if (BufferIsValid(childbuf))
> PageSetLSN(childpage, recptr);
> +
> + if (stack->parent != NULL && !GinPageIsLeaf(newlpage))
> + {
> + XLogFlush(recptr);
> + elog(PANIC, "internal page split for block: %u",
> stack->blkno);
> + }
> }
> END_CRIT_SECTION();
> ```
>
> With the modifications above, the problem can be reproduced with the
> following
> steps:
> ```
> alter system set autovacuum to off;
> alter system set gin_pending_list_limit to 64;
> select pg_reload_conf();
> create table t (a text);
> create extension btree_gin;
> create index on t USING gin (a);
> -- PANIC crash
> insert into t select generate_series(1, 100000);
>
> -- Assert fail
> insert into t select 1;
> ```
>
> I reviewed all places where ginFinishSplit is called, and only in two
> instances
> in ginFindLeafPage might it be possible to not hold an exclusive lock on
> stack->buffer.
>
> The patch I provided in the attachment has been verified to fix the issue
> mentioned above.
> However, it may not be perfect: passing GIN_EXCLUSIVE to ginStepRight might
> affect performance. Do we need to add a parameter to ginStepRight, and
> within
> the function ginStepRight, elevate the lock level for an incomplete
> split page?
>
> Looking forward to suggestions from developers!
Thanks, I'll look into this. The fix seems fine at a quick glance, but
I'll think about the performance aspect a bit more.
I'm thinking of adding tests for this using the new fault-injection
facility that Michael just merged in commit d86d20f0ba. For GIN, but
also B-tree which has a similar incomplete-split mechanism.
Another way to create a scenario with incomplete splits, which doesn't
involve any crashes or errors, would be to perform PITR to just between
the insert and the finish-split records. But the fault-injection seems
easier.
--
Heikki Linnakangas
Neon (https://neon.tech)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: hlinnaka@iki.fi, feichanghong@qq.com, pgsql-bugs@lists.postgresql.org
Subject: Re: MarkBufferDirty Assert held LW_EXCLUSIVE lock fail when ginFinishSplit
In-Reply-To: <053bc845-3cb5-445e-8ea4-00f6c678ee25@iki.fi>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox