Re: RecoveryInProgress() has critical side effects

On 17/11/2021 00:04, Andres Freund wrote:
> On 2021-11-16 16:30:27 -0500, Robert Haas wrote:
>> I'm still not entirely clear on whether you prefer v1-0002, v2-0002,
>> or something else.
>
> I think it basically doesn't matter much. It's such a small piece of the cost
> compared to either the cost of a single insert or the ratio between
> RedoRecPtr/FPW changes and the number of inserted records.
>
> I guess v2-0002 is mildly simpler, so I very weakly lean towards that.

I was going to argue for v1, because I was worried about generating
full-page writes unnecessarily. But looking closer, it doesn't do that.
It will initially think that no full page images are needed, and perform
the extra loop in XLogInsert() to try again with full page images.
Similarly in heap_update(), it will fail in the direction of sometimes
doing the "prefix-suffix compression" unnecessarily, which means a
little bit of extra CPU work, but it won't affect the WAL record that
gets written.

So either way is fine, really. That's a bit subtle, though, a comment
somewhere would be good.

But here's yet another idea: We could initialize RedoRecPtr and
doPageWrites in InitXLogInsert(). It would seem like a pretty natural
place for it.

- Heikki

PS. typo in patch v2: s/prepard/prepared/