On 2021-May-13, Tom Lane wrote: > Alvaro Herrera <alvhe...@alvh.no-ip.org> writes: > > (Looking again, the nbtpage.c hunk might have been made obsolete by > > c34787f91058 and other commits). > > OK. Here's a revision that adopts your idea, except that I left > out the nbtpage.c change since you aren't sure of that part.
Thanks. > I added a macro that allows spgdoinsert to Assert that it's not > called in a context where the infinite-loop-due-to-InterruptPending > risk would arise. This is a little bit fragile, because it'd be > easy for ill-considered changes to ProcessInterrupts to break it, > but it's better than nothing. Hmm, it looks OK to me, but I wonder why you kept the original CHECK_FOR_INTERRUPTS()s since these would be done once we've broken out of the loop anyway. I tested Dilip's original test case and while we still die on OOM, we're able to interrupt it before dying. Not related to this patch -- I was bothered by the UnlockReleaseBuffer calls at the bottom of spgdoinsert that leave the buffer still set in the structs. It's not a problem if you look only at this routine, but I notice that callee doPickSplit does the same thing, so maybe there is some weird situation in which you could end up with the buffer variable set, but we don't hold lock nor pin on the page, so an attempt to clean up would break. I don't know enough about spgist to figure out how to craft a test case, maybe it's impossible to reach for some reason, but it seems glass-in-the-beach sort of thing. -- Álvaro Herrera 39°49'30"S 73°17'W "Someone said that it is at least an order of magnitude more work to do production software than a prototype. I think he is wrong by at least an order of magnitude." (Brian Kernighan)