Thank you for the comment. At Tue, 26 May 2020 20:17:47 +0800, Craig Ringer <cr...@2ndquadrant.com> wrote in > On Tue, 26 May 2020, 15:40 Kyotaro Horiguchi, <horikyota....@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
