Hi,
On 2022-12-02 14:22:55 +0900, Michael Paquier wrote:
> On Fri, Nov 04, 2022 at 09:52:39AM +0900, Ian Lawrence Barwick wrote:
> > CommitFest 2022-11 is currently underway, so if you are interested
> > in moving this patch forward, now would be a good time to update it.
>
> No replies after 4 weeks, so I have marked this entry as returned
> with feedback. I am still wondering what would be the best thing to
> do here..
IMO this a bugfix, I don't think we can just close the entry, even if Matthias
doesn't have time / energy to push it forward.
I think the big issue with the patch as it stands is that it will typically
cause PANICs on failure, because the record-too-large ERROR be a in a critical
section. That's still better than generating a record that can't be replayed,
but it's not good.
There's not all that many places with potentially huge records. I wonder if we
ought to modify at least the most prominent ones to prepare the record before
the critical section. I think the by far most prominent real-world case is
RecordTransactionCommit(). I think we could rename XactLogCommitRecord() to
XactBuildCommitRecord() build the commit record, then have the caller do
START_CRIT_SECTION(), set DELAY_CHKPT_START, and only then do the
XLogInsert().
That'd even have the nice side-effect of reducing the window in which
DELAY_CHKPT_START is set a bit.
Greetings,
Andres Freund
