Laetitia Avrot <laetitia.avrot@gmail.com> writes:
> [ v3-0001-update-table-stats-after-commit-and-chain.patch ]
I took a quick look at this. If we're going to move the responsibility
for pushing out stats to transaction commit, then I think what we want
to do is put the call somewhere near the end of CommitTransaction.
Maybe right after AtCommit_Notify, because that has the same spirit
of sending out info about our having committed to other backends.
(The pgstat_report_xact_timestamp call would need to be moved up to
before that, I think, but that sure looks like it was inserted with
the aid of a dartboard anyway. It certainly doesn't square with the
immediately preceding comment about "purely internal" cleanup.)
AbortTransaction and probably PrepareTransaction the same.
Having done that, rather than adding more pgstat_report_stat calls
we could get rid of most of them. I see a bunch of random calls right
after various CommitTransactionCommand calls, which we'd not need anymore.
We probably still want the one in PostgresMain, but it needs some
rethinking, because we only want that one to do something when the timer
fires saying that we've slept too long without sending stats.
As for the NOTIFY business, I'm not impressed with putting duplicate
boilerplate into half a dozen different places. I think the discussion
in ref/notify.sgml covers this already, or if not, that's the one place
to fix it. The place I actually thought could use more attention is the
wire protocol spec in protocol.sgml. In any case, I'd rather see that
as a separate patch, because it's unrelated.
regards, tom lane
