Hi, On 2022-05-12 22:26:51 +0100, Simon Riggs wrote: > On Thu, 12 May 2022 at 04:40, Andres Freund <and...@anarazel.de> wrote: > > I'm not happy with the idea of random code being executed in the middle of > > CheckPointGuts(), without any documentation of what is legal to do at that > > point. > > The "I'm not happy.." ship has already sailed with pluggable rmgrs.
I don't agree. The ordering within a checkpoint is a lot more fragile than what you do in an individual redo routine. > Checkpoints exist for one purpose - as the starting place for recovery. > > Why would we allow pluggable recovery without *also* allowing > pluggable checkpoints? Because one can do a lot of stuff with just pluggable WAL records, without integrating into checkpoints? Note that I'm *not* against making checkpoint extensible - I just think it needs a good bit of design work around when the hook is called etc. I definitely think it's too late in the cycle to add checkpoint extensibility now. > > To actually be useful we'd likely need multiple calls to such an rmgr > > callback, with a parameter where in CheckPointGuts() we are. Right now the > > sequencing is explicit in CheckPointGuts(), but with the proposed callback, > > that'd not be the case anymore. > > It is useful without the extra complexity you mention. Shrug. The documentation work definitely is needed. Perhaps we can get away without multiple callbacks within a checkpoint, I think it'll become more apparent when writing information about the precise point in time the checkpoint callback is called. > I see multiple uses for the rm_checkpoint() point proposed and I've > been asked multiple times for a checkpoint hook. Any rmgr that > services crash recovery for a non-smgr based storage system would need > this because the current checkpoint code only handles flushing to disk > for smgr-based approaches. That is orthogonal to other code during > checkpoint, so it stands alone quite well. FWIW, for that there are much bigger problems than checkpoint extensibility. Most importantly there's currently no good way to integrate relation creation / drop with the commit / abort infrastructure... Greetings, Andres Freund