Hi Andrey, > the test passes because you moved injection point to a very safe position > [...] > I want to emphasize that it seems to me that position of injection point is > not a hint, but rather coincidental.
Well, I wouldn't say that the test passes merely because the location of the injection point was moved. For sure it was moved, because the visibilitymap_clear() call was moved. Maybe I misunderstood the intent of the test. Wasn't it to call the injection point right after updating the VM? I tried to place it between updating the WAL and updating the VM and the effect was the same - the test still passes. In any case we can place it anywhere we want to if we agree to include the test into the final version of the patch. > I concur that all other users of visibilitymap_clear() likely will need to be > fixed. Right, I realized there are a few places besides heapam.c that might need a change. > The approach seems viable to me, but I'd like to have understanding why > PD_ALL_VISIBLE in a heap page header did not save the day before fixing > anything > ... But only when we have a good picture what exactly is broken. Agree. I especially would like to know the opinion of somebody who's been hacking Postgres longer than I did. Perhaps there was a good reason to update the VM *before* creating WAL records I'm unaware of.
