Hi, Replying on the new thread. Original message at https://www.postgresql.org/message-id/CAA4eK1%2BH2m95HhzfpRkwv2-GtFwtbcVp7837X49%2Bvs0RXX3dBA%40mail.gmail.com
On 2023-02-09 15:54:19 +0530, Amit Kapila wrote: > One thing to note about the changes we are discussing here is that > some of the plugins like wal2json already call > OutputPluginUpdateProgress in their commit callback. They may need to > update it accordingly. It was a fundamental mistake to add OutputPluginUpdateProgress(). I don't like causing unnecessary breakage, but this seems necessary. > One difference I see with the patch is that I think we will end up > sending keepalive for empty prepared transactions even though we don't > skip sending begin/prepare messages for those. With the proposed approach we reliably know whether a callback wrote something, so we can tune the behaviour here fairly easily. Likely WalSndUpdateProgress() should not do anything if did_write && !finished_xact. > The reason why we don't skip sending prepare for empty 2PC xacts is that if > the WALSender restarts after the PREPARE of a transaction and before the > COMMIT PREPARED of the same transaction then we won't be able to figure out > if we have skipped sending BEGIN/PREPARE of a transaction. It's probably not a good idea to skip sending 2PC state changes anyway, at least when used for replication, rather than CDC type use cases. But I again think that that's not something the core system can assume. I'm sad that we went so far down a pretty obviously bad rabbit hole. Adding incrementally more of the progress calls to pgoutput, and knowing that wal2json also added some, should have run some pretty large alarm bells. > To skip sending prepare for empty xacts, we previously thought of some ideas > like (a) At commit-prepare time have a check on the subscriber-side to know > whether there is a corresponding prepare for it before actually doing > commit-prepare but that sounded costly. (b) somehow persist the information > whether the PREPARE for a xact is already sent and then use that information > for commit prepared but again that also didn't sound like a good idea. I don't think it's worth optimizing this. However, the explanation for why we're not skipping empty prepared xacts needs to be added to pgoutput_prepare_txn() etc. Greetings, Andres Freund
