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
