On Mon, Feb 19, 2024 at 3:56 PM Michael Paquier <mich...@paquier.xyz> wrote: > > On Mon, Feb 19, 2024 at 10:26:43AM +0100, Tomas Vondra wrote: > > On 2/19/24 07:57, Michael Paquier wrote: > > > On Sun, Feb 18, 2024 at 02:28:06AM +0100, Tomas Vondra wrote: > >>> 1) Do we really need to modify RecordTransactionCommitPrepared and > >>> XactLogCommitRecord to return the LSN of the commit record? Can't we > >>> just use XactLastRecEnd? It's good enough for advancing > >>> replorigin_session_origin_lsn, why wouldn't it be good enough for this? > >> > >> Or XactLastCommitEnd? > > > > But that's not set in RecordTransactionCommitPrepared (or twophase.c in > > general), and the patch seems to need that. > > Hmm. Okay. > > > > The changes in AtEOXact_PgStat() are not really > > > attractive for what's a static variable in all the backends. > > > > I'm sorry, I'm not sure what "changes not being attractive" means :-( > > It just means that I am not much a fan of the signature changes done > for RecordTransactionCommit() and AtEOXact_PgStat_Database(), adding a > dependency to a specify LSN. Your suggestion to switching to > XactLastRecEnd should avoid that.
Attached is an updated patch that uses XactLastCommitEnd and therefore avoids all of those signature changes. We can't use XactLastCommitEnd because it gets set to 0 immediately after RecordTransactionCommit() sets XactLastCommitEnd to its value. I added a test for two-phase commit, as well, and in so doing I discovered something that I found curious: when I do "COMMIT PREPARED 't1'", not only does RecordTransactionCommitPrepared() get called, but eventually RecordTransactionCommit() is called as well before the command returns (despite the comments for those functions describing them as two equivalents you need to change at the same time). So it appears that we don't need to set XactLastCommitEnd in RecordTransactionCommitPrepared() for this to work, and, indeed, adding some logging to verify, the value of XactLastRecEnd we'd use to update XactLastCommitEnd is the same at the end of both of those functions during COMMIT PREPARED. I'd like to have someone weigh in on whether relying on RecordTransactionCommit() being called during COMMIT PREPARED is correct or not (if perchance there are non-obvious reasons why we shouldn't). Regards, James Coleman
v4-0001-Add-last-commit-s-LSN-to-pg_stat_database.patch
Description: Binary data
