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
savethe 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.