public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
To: Heikki Linnakangas <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Alexander Kuzmenkov <[email protected]>
Cc: Ashutosh Sharma <[email protected]>
Subject: Re: Assertion failure in hash_kill_items()
Date: Tue, 17 Mar 2026 13:40:10 -0400
Message-ID: <p45puiwvjtbeff6cx2suwk3i35zubdot6iaa3z6buiqnoix7v6@hzygqumsgome> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
Hi,
On 2026-03-17 19:15:10 +0200, Heikki Linnakangas wrote:
> I bumped into an assertion failure, while playing with variants of the test
> case that Alexander Kuzmenkov wrote to exercise hash index page cleanup [1].
> This is master-only, related to the recent changes in how buffers are marked
> dirty.
Sorry, I had hoped to push a fix for that already, after it was reported in
https://postgr.es/m/vjtmvwvbxt7w5uyacxpzibpj65ewcb7uqaqbhd4arvnjbp5jqz%405ksdh6fsyqve
but real life intervened.
I was planning to commit it together with an addition to
src/test/modules/index/specs/killtuples.spec
Unfortunately that made the change a good bit more verbose, as a naive
addition would report a number of buffer accesses that seemed not necessarily
reliable to me. So I updated the 'result' step to just return true/false
depending on whether there were any accesses.
I'll go and work on pushing that.
> The first attached patch fixes it. It's pretty straightforward: the function
> was using so->currPos.buf, but that's only valid if the page was already
> pinned on entry to the function. It should be using the local 'buf' variable
> instead.
Yea, that was a stupid bug on my part. No idea how I ended up with it. At
first I thought it might have been a rebase issue, but I didn't see any
relevant change that could have caused that.
> The second patch simplifies the condition in the 'unlock_page' part. This
> isn't new, and isn't needed to fix the bug, it just caught my eye while
> looking at this. I don't understand why the condition was the way it was,
> checking just 'havePin' seems sufficient and more correct to me. Am I
> missing something?
I can't see anything either, quite odd. Most likely explanation seems to be
that something changed during the development of 7c75ef571579.
Indeed, the first version of the patch from
https://postgr.es/m/CAE9k0Pm3KTx93K8_5j6VMzG4h5F%2BSyknxUwXrN-zqSZ9X8ZS3w%40mail.gmail.com
was using "if (so->hashso_bucket_buf == so->currPos.buf)" both at the start
and end of _hash_kill_items(). So likely it was just an accident during patch
revisions.
Greetings,
Andres Freund
view thread (4+ messages) latest in thread
reply
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Assertion failure in hash_kill_items()
In-Reply-To: <p45puiwvjtbeff6cx2suwk3i35zubdot6iaa3z6buiqnoix7v6@hzygqumsgome>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox