From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: [BUG] Error in BRIN summarization
Date: Tue, 11 Aug 2020 19:43:21 -0400
Message-ID: <20200811234321.GA17597@alvherre.pgsql> (raw)
In-Reply-To: <2588667e-d07d-7e10-74e2-7e1e46194491@postgrespro.ru>
On 2020-Jul-30, Anastasia Lubennikova wrote:
> While testing this fix, Alexander Lakhin spotted another problem. I
> simplified the test case to this:
Ah, good catch. I think a cleaner way to fix this problem is to just
consider the range as not summarized and return NULL from there, as in
the attached patch. Running your test case with a telltale WARNING
added at that point, it's clear that it's being hit.
By returning NULL, we're forcing the caller to scan the heap, which is
not great. But note that if you retry, and your VACUUM hasn't run yet
by the time we go through the loop again, the same thing would happen.
So it seems to me a good enough answer.
A much more troubling thought is what happens if the range is
desummarized, then the index item is used for the summary of a different
range. Then the index might end up returning corrupt results.
> At first, I tried to fix it by holding the lock on revmap->rm_currBuf until
> we locked the regular page, but it causes a deadlock with brinsummarize(),
> It can be easily reproduced with the same test as above.
> Is there any rule about the order of locking revmap and regular pages in
> brin? I haven't found anything in README.
Umm, I thought that stuff was in the README, but it seems I didn't add
it there. I think I had a .org file with my notes on that ... must be
in an older laptop disk, because it's not in my worktree for that. I'll
see if I can fish it out.
--
Ălvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:brin-desumm-race.patch (text/x-diff, 907B) downloaddiff --git a/src/backend/access/brin/brin_revmap.c b/src/backend/access/brin/brin_revmap.cindex e8b8308f82..35746714a7 100644--- a/src/backend/access/brin/brin_revmap.c+++ b/src/backend/access/brin/brin_revmap.c@@ -282,10 +282,17 @@ brinGetTupleForHeapBlock(BrinRevmap *revmap, BlockNumber heapBlk,
/* If we land on a revmap page, start over */
if (BRIN_IS_REGULAR_PAGE(page))
{
+ /*+ * If the offset number is greater than what's in the page, it's+ * possible that the range was desummarized concurrently. Just+ * return NULL to handle that case.+ */
if (*off > PageGetMaxOffsetNumber(page))
- ereport(ERROR,- (errcode(ERRCODE_INDEX_CORRUPTED),- errmsg_internal("corrupted BRIN index: inconsistent range map")));+ {+ LockBuffer(*buf, BUFFER_LOCK_UNLOCK);+ return NULL;+ }+
lp = PageGetItemId(page, *off);
if (ItemIdIsUsed(lp))
{
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-hackers@postgresql.org
Cc: alvherre@2ndquadrant.com, a.lubennikova@postgrespro.ru, pgsql-hackers@lists.postgresql.org
Subject: Re: [BUG] Error in BRIN summarization
In-Reply-To: <20200811234321.GA17597@alvherre.pgsql>
* 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