On Fri, 13 May 2022 at 00:42, Andres Freund <andres@anarazel.de> wrote:
>
> On 2022-05-12 22:26:51 +0100, Simon Riggs wrote:
> > On Thu, 12 May 2022 at 04:40, Andres Freund <andres@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.
Example?
> > 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.
When was any such work done previously for any other hook?? That isn't needed.
Checkpoints aren't complete until all data structures have
checkpointed, so there are no problems from a partial checkpoint being
written.
As a result, the order of actions in CheckpointGuts() is mostly
independent of each other. The SLRUs are all independent of each
other, as is CheckPointBuffers().
The use cases I'm trying to support aren't tricksy modifications of
existing code, they are just entirely new data structures which are
completely independent of other Postgres objects.
> 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.
You seem to be thinking in terms of modifying the existing actions in
CheckpointGuts(). I don't care about that. Anybody that wishes to do
that can work out the details of their actions.
There is nothing to document, other than "don't do things that won't
work". How can anyone enumerate all the things that wouldn't work??
There is no list of caveats for any other hook. Why is it needed here?
> > 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...
One at a time...
--
Simon Riggs http://www.EnterpriseDB.com/