Hi, On 2021-03-25 10:07:28 +0100, Markus Wanner wrote: > This leads to the possibility of the transaction getting aborted concurrent > to logical decoding. In that case, it is likely for the decoder to error on > a catalog scan that conflicts with the abort of the transaction. The > reorderbuffer sports a PG_CATCH block to cleanup.
FWIW, I am seriously suspicuous of the code added as part of 7259736a6e5b7c75 and plan to look at it after the code freeze. I can't really see this code surviving as is. The tableam.h changes, the bsyscan stuff, ... Leaving correctness aside, the code bloat and performance affects alone seems problematic. > Again, this callback is especially important for output plugins that invoke > further actions on downstream nodes that delay the COMMIT PREPARED of a > transaction upstream, e.g. until prepared on other nodes. Up until now, the > output plugin has no way to learn about a concurrent abort of the currently > decoded (2PC or streamed) transaction (perhaps short of continued polling on > the transaction status). You may have only meant it as a shorthand: But imo output plugins have absolutely no business "invoking actions downstream". > diff --git a/src/backend/replication/logical/reorderbuffer.c > b/src/backend/replication/logical/reorderbuffer.c > index c291b05a423..a6d044b870b 100644 > --- a/src/backend/replication/logical/reorderbuffer.c > +++ b/src/backend/replication/logical/reorderbuffer.c > @@ -2488,6 +2488,12 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, > ReorderBufferTXN *txn, > errdata = NULL; > curtxn->concurrent_abort = true; > > + /* > + * Call the cleanup hook to inform the output plugin > that the > + * transaction just started had to be aborted. > + */ > + rb->concurrent_abort(rb, txn, streaming, commit_lsn); > + > /* Reset the TXN so that it is allowed to stream > remaining data. */ > ReorderBufferResetTXN(rb, txn, snapshot_now, > command_id, > prev_lsn, I don't think this would be ok, errors thrown in the callback wouldn't be handled as they would be in other callbacks. Greetings, Andres Freund
