Hi, On 2023-02-13 08:22:34 +0530, Amit Kapila wrote: > On Sat, Feb 11, 2023 at 3:04 AM Andres Freund <and...@anarazel.de> wrote: > > > > > 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. > > > > I would like to clarify a few things about the proposed approach. In > commit_cb_wrapper()/prepare_cb_wrapper(), the patch first did > ctx->did_write = false;, then call the commit/prepare callback (which > will call pgoutput_commit_txn()/pgoutput_prepare_txn()) and then call > update_progress() which will make decisions based on ctx->did_write > flag. Now, for this to work pgoutput_commit_txn/pgoutput_prepare_txn > should know that the transaction has performed some writes before that > call which is currently working because pgoutput is tracking the same > via sent_begin_txn.
I don't really see these as being related. What pgoutput does internally to optimize for some usecases shouldn't matter to the larger infrastructure. > Is the intention here that we still track whether BEGIN () has been sent via > pgoutput? Yes. If somebody later wants to propose tracking this alongside a txn and passing that to the output plugin callbacks, we can do that. But that's independent of fixing the broken architecture of the progress infrastructure. Greetings, Andres Freund
