Thank you for reviewing this patch. I won't have time to take into account your comments before the end of this commit great but will try my best to make it for the next one.
I'll change the patch status to "moved to next CF".
Have a nice day,
Lætitia
Le jeu. 28 juil. 2022, 23:26, Tom Lane <tgl@sss.pgh.pa.us> a écrit :
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.
