Hi,

On 2025-12-11 10:29:42 +0530, Ashutosh Bapat wrote:
> Please review.

I'd simplify the patch to, initially, to just track the sent bytes.  For one,
that's by *far* the most useful statistic.  But I also have some concerns
about the other stats:


- To me filteredBytes is a pretty bogus number - the size that the output
  plugin would have sent and ReorderBufferChangeSize() are only kinda
  related. It'll be a hard number to interpret, I think.

  It also seems to not account for filtering that happens based on the origin
  id.


- I don't have fundamental opposition to tracking the number of sent
  transactions, but I think the implementation is at the wrong place.

  I think we ought to add explicit support for output plugins to filter
  transactions. Calling output plugins once for each change in a large
  transaction, which we already decided to not send out, makes no sense. It's
  far from free to do all the setup to decode a tuple, if the transaction is
  filtered, we shouldn't do that.

  It's also far far from free to restore changes from disk if we are going to
  throw away the whole transaction.

  The current way requires each output plugin to maintain its own tracking
  about whether it decided to not output the transaction, which doesn't seem
  right to me.

  I'm also just not sure how useful it is, because most of the time we're
  going to filter on a per-change basis (e.g. only rels in a publication). But
  that's only sometimes going to affect the numbers of sent transactions.


  I'm not convinced as-is it's worth the breakage of all output plugins.


Greetings,

Andres


Reply via email to