On Thu, Oct 26, 2023 at 12:38 PM Hayato Kuroda (Fujitsu) <kuroda.hay...@fujitsu.com> wrote: > > > PFA v25 patch set. The changes are: > > Thanks for making the patch! It seems that there are lots of comments, so > I can put some high-level comments for 0001. > Sorry if there are duplicated comments. > > 1. > The patch seemed not to consider the case that failover option between > replication > slot and subscription were different. Currently slot option will be > overwritten > by subscription one. > > Actually, I'm not sure what specification is better. Regarding the two_phase, > 2PC will be decoded only when the both of settings are true. Should we follow? > > 2. > Currently ctx->failover is set only in the pgoutput_startup(), but not sure > it is OK. > Can we change the parameter in CreateDecodingContext() or similar functions? > > Because IIUC it means that only slots which have pgoutput can wait. Other > output plugins must understand the change and set faliover flag as well - > I felt it is not good. E.g., you might miss to enable the parameter in > test_decoding. > > Regarding the two_phase parameter, setting on plugin layer is good because it > quite affects the output. As for the failover, it is not related with the > content so that all of slots should be enabled. >
Both of your points seem valid to me. However, I think they should be addressed once we make option 'failover' behave similar to the '2PC' option as per discussion [1]. [1] - https://www.postgresql.org/message-id/b099ebc2-68fd-4c08-87ce-65fc4cb24121%40gmail.com -- With Regards, Amit Kapila.