Thank you for the comment.
At Tue, 26 May 2020 20:17:47 +0800, Craig Ringer <craig@2ndquadrant.com> wrote in
> On Tue, 26 May 2020, 15:40 Kyotaro Horiguchi, <horikyota.ntt@gmail.com>
> wrote:
>
> >
> > This patch removes all the three callbacks (open/close/page_read) in
> > XL_ROUTINE from XLogReaderState. It only has "cleanup" callback
> > instead.
> >
>
> I actually have a use in mind for these callbacks - to support reading WAL
> for logical decoding from a restore_command like tool. So we can archive
> wal when it's no longer required for recovery and reduce the risk of
> filling pg_wal if a standby lags.
>
> I don't object to your cleanup at all. I'd like it to be properly
> pluggable, whereas right now it has hard coded callbacks that differ for
> little reason.
>
> Just noting that the idea of a callback here isn't a bad thing.
I agree that plugin is generally not bad as far as it were standalone,
that is, as far as it is not tightly cooperative with the opposite
side of the caller of it. However, actually it seems to me that the
xlogreader plugins are too-deeply coupled with the callers of
xlogreader in many aspects involving error-handling and
retry-mechanism.
As Alvaro mentioned we may have page-decrypt callback shortly as
another callback of xlogreader. Xlogreader could be more messy by
introducing such plugins, that actually have no business with
xlogreader at all.
Evidently xlogreader can be a bottom-end module (that is, a module
that doesn't depend on another module). It is I think a good thing to
isolate xlogreader from the changes of its callers and correlated
plugins.
A major problem of this patch is that the state machine used there
might be another mess here, though.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center