public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 03/10] Make sure published XIDs are persistent 3+ messages / 3 participants [nested] [flat]
* [PATCH 03/10] Make sure published XIDs are persistent @ 2021-03-08 06:43 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Kyotaro Horiguchi @ 2021-03-08 06:43 UTC (permalink / raw) pg_xact_status() premises that XIDs obtained by pg_current_xact_id(_if_assigned)() are persistent beyond a crash. But XIDs are not guaranteed to go beyond WAL buffers before commit and thus XIDs may vanish if server crashes before commit. This patch guarantees the XID shown by the functions to be flushed out to disk. Copied from: https://www.postgresql.org/message-id/20210308.173242.463790587797836129.horikyota.ntt%40gmail.com --- src/backend/access/transam/xact.c | 55 +++++++++++++++++++++++++------ src/backend/access/transam/xlog.c | 2 +- src/backend/utils/adt/xid8funcs.c | 12 ++++++- src/include/access/xact.h | 3 +- 4 files changed, 59 insertions(+), 13 deletions(-) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 6395a9b240..38e978d238 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -201,7 +201,7 @@ typedef struct TransactionStateData int prevSecContext; /* previous SecurityRestrictionContext */ bool prevXactReadOnly; /* entry-time xact r/o state */ bool startedInRecovery; /* did we start in recovery? */ - bool didLogXid; /* has xid been included in WAL record? */ + XLogRecPtr minLSN; /* LSN needed to reach to record the xid */ int parallelModeLevel; /* Enter/ExitParallelMode counter */ bool chain; /* start a new block after this one */ bool assigned; /* assigned to top-level XID */ @@ -520,14 +520,46 @@ GetCurrentFullTransactionIdIfAny(void) * MarkCurrentTransactionIdLoggedIfAny * * Remember that the current xid - if it is assigned - now has been wal logged. + * + * upto is the LSN up to which we need to flush WAL to ensure the current xid + * to be persistent. See EnsureCurrentTransactionIdLogged(). */ void -MarkCurrentTransactionIdLoggedIfAny(void) +MarkCurrentTransactionIdLoggedIfAny(XLogRecPtr upto) { - if (FullTransactionIdIsValid(CurrentTransactionState->fullTransactionId)) - CurrentTransactionState->didLogXid = true; + if (FullTransactionIdIsValid(CurrentTransactionState->fullTransactionId) && + XLogRecPtrIsInvalid(CurrentTransactionState->minLSN)) + CurrentTransactionState->minLSN = upto; } +/* + * EnsureCurrentTransactionIdLogged + * + * Make sure that the current top XID is WAL-logged. + */ +void +EnsureTopTransactionIdLogged(void) +{ + /* + * We need at least one WAL record for the current top transaction to be + * flushed out. Write one if we don't have one yet. + */ + if (XLogRecPtrIsInvalid(TopTransactionStateData.minLSN)) + { + xl_xact_assignment xlrec; + + xlrec.xtop = XidFromFullTransactionId(XactTopFullTransactionId); + Assert(TransactionIdIsValid(xlrec.xtop)); + xlrec.nsubxacts = 0; + + XLogBeginInsert(); + XLogRegisterData((char *) &xlrec, MinSizeOfXactAssignment); + TopTransactionStateData.minLSN = + XLogInsert(RM_XACT_ID, XLOG_XACT_ASSIGNMENT); + } + + XLogFlush(TopTransactionStateData.minLSN); +} /* * GetStableLatestTransactionId @@ -616,14 +648,14 @@ AssignTransactionId(TransactionState s) * When wal_level=logical, guarantee that a subtransaction's xid can only * be seen in the WAL stream if its toplevel xid has been logged before. * If necessary we log an xact_assignment record with fewer than - * PGPROC_MAX_CACHED_SUBXIDS. Note that it is fine if didLogXid isn't set + * PGPROC_MAX_CACHED_SUBXIDS. Note that it is fine if minLSN isn't set * for a transaction even though it appears in a WAL record, we just might * superfluously log something. That can happen when an xid is included * somewhere inside a wal record, but not in XLogRecord->xl_xid, like in * xl_standby_locks. */ if (isSubXact && XLogLogicalInfoActive() && - !TopTransactionStateData.didLogXid) + XLogRecPtrIsInvalid(TopTransactionStateData.minLSN)) log_unknown_top = true; /* @@ -693,6 +725,7 @@ AssignTransactionId(TransactionState s) log_unknown_top) { xl_xact_assignment xlrec; + XLogRecPtr endptr; /* * xtop is always set by now because we recurse up transaction @@ -707,11 +740,13 @@ AssignTransactionId(TransactionState s) XLogRegisterData((char *) unreportedXids, nUnreportedXids * sizeof(TransactionId)); - (void) XLogInsert(RM_XACT_ID, XLOG_XACT_ASSIGNMENT); + endptr = XLogInsert(RM_XACT_ID, XLOG_XACT_ASSIGNMENT); nUnreportedXids = 0; - /* mark top, not current xact as having been logged */ - TopTransactionStateData.didLogXid = true; + + /* set minLSN of top, not of current xact if not yet */ + if (XLogRecPtrIsInvalid(TopTransactionStateData.minLSN)) + TopTransactionStateData.minLSN = endptr; } } } @@ -2022,7 +2057,7 @@ StartTransaction(void) * initialize reported xid accounting */ nUnreportedXids = 0; - s->didLogXid = false; + s->minLSN = InvalidXLogRecPtr; /* * must initialize resource-management stuff first diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 15da91a8dd..6eb46ea8a7 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -1172,7 +1172,7 @@ XLogInsertRecord(XLogRecData *rdata, */ WALInsertLockRelease(); - MarkCurrentTransactionIdLoggedIfAny(); + MarkCurrentTransactionIdLoggedIfAny(EndPos); END_CRIT_SECTION(); diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c index cc2b4ac797..992482f8c8 100644 --- a/src/backend/utils/adt/xid8funcs.c +++ b/src/backend/utils/adt/xid8funcs.c @@ -357,6 +357,8 @@ bad_format: Datum pg_current_xact_id(PG_FUNCTION_ARGS) { + FullTransactionId xid; + /* * Must prevent during recovery because if an xid is not assigned we try * to assign one, which would fail. Programs already rely on this function @@ -365,7 +367,12 @@ pg_current_xact_id(PG_FUNCTION_ARGS) */ PreventCommandDuringRecovery("pg_current_xact_id()"); - PG_RETURN_FULLTRANSACTIONID(GetTopFullTransactionId()); + xid = GetTopFullTransactionId(); + + /* the XID is going to be published, make sure it is psersistent */ + EnsureTopTransactionIdLogged(); + + PG_RETURN_FULLTRANSACTIONID(xid); } /* @@ -380,6 +387,9 @@ pg_current_xact_id_if_assigned(PG_FUNCTION_ARGS) if (!FullTransactionIdIsValid(topfxid)) PG_RETURN_NULL(); + /* the XID is going to be published, make sure it is psersistent */ + EnsureTopTransactionIdLogged(); + PG_RETURN_FULLTRANSACTIONID(topfxid); } diff --git a/src/include/access/xact.h b/src/include/access/xact.h index 34cfaf542c..a61e4d6da5 100644 --- a/src/include/access/xact.h +++ b/src/include/access/xact.h @@ -386,7 +386,8 @@ extern FullTransactionId GetTopFullTransactionId(void); extern FullTransactionId GetTopFullTransactionIdIfAny(void); extern FullTransactionId GetCurrentFullTransactionId(void); extern FullTransactionId GetCurrentFullTransactionIdIfAny(void); -extern void MarkCurrentTransactionIdLoggedIfAny(void); +extern void MarkCurrentTransactionIdLoggedIfAny(XLogRecPtr upto); +extern void EnsureTopTransactionIdLogged(void); extern bool SubTransactionIsActive(SubTransactionId subxid); extern CommandId GetCurrentCommandId(bool used); extern void SetParallelStartTimestamps(TimestampTz xact_ts, TimestampTz stmt_ts); -- 2.17.0 --XsQoSWH+UP9D9v3l Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0004-wal_compression_method-default-to-zlib.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Doc update for pg_stat_statements normalization @ 2023-02-28 23:11 Imseih (AWS), Sami <[email protected]> 2023-03-01 00:09 ` Re: Doc update for pg_stat_statements normalization Michael Paquier <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Imseih (AWS), Sami @ 2023-02-28 23:11 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> > I am OK with an addition to the documentation to warn that one may > have to increase the maximum number of entries that can be stored if > seeing a non-normalized entry that should have been normalized. I agree. We introduce the concept of a plannable statement in a previous section and we can then make this distinction in the new paragraph. I also added a link to pg_stat_statements_info since that is introduced later on int the doc. Regards, -- Sami Imseih Amazon Web Services Attachments: [application/octet-stream] 0001-doc-update-regarding-pg_stat_statements-normalizatio.patch (1.5K, ../../[email protected]/2-0001-doc-update-regarding-pg_stat_statements-normalizatio.patch) download | inline diff: From 4b52c3d6dd142264ba29ac592a881232492d6d89 Mon Sep 17 00:00:00 2001 From: "Imseih (AWS)" <[email protected]> Date: Tue, 28 Feb 2023 16:39:40 -0600 Subject: [PATCH 1/1] doc update regarding pg_stat_statements normalization. It is quite possible that a query text willl not normalize (replace constants with $1, $2..) when there is a high rate pgss_hash deallocation. This commit calls this out in docs. --- doc/src/sgml/pgstatstatements.sgml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml index efc36da602..39e2f8eaec 100644 --- a/doc/src/sgml/pgstatstatements.sgml +++ b/doc/src/sgml/pgstatstatements.sgml @@ -516,6 +516,16 @@ <structname>pg_stat_statements</structname> entry. </para> + <para> + Text for a plannable query may be observed with constants in + <structname>pg_stat_statements</structname>, especially when there is a high + rate of entry deallocations. To reduce the likelihood of this happening, consider + increasing <varname>pg_stat_statements.max</varname>. + The <structname>pg_stat_statements_info</structname> view, discussed below + in <xref linkend="pgstatstatements-pg-stat-statements-info"/>, + provides entry deallocation statistics. + </para> + <para> In some cases, queries with visibly different texts might get merged into a single <structname>pg_stat_statements</structname> entry. Normally this will happen -- 2.37.1 (Apple Git-137.1) ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Doc update for pg_stat_statements normalization 2023-02-28 23:11 Re: Doc update for pg_stat_statements normalization Imseih (AWS), Sami <[email protected]> @ 2023-03-01 00:09 ` Michael Paquier <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Michael Paquier @ 2023-03-01 00:09 UTC (permalink / raw) To: Imseih (AWS), Sami <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> On Tue, Feb 28, 2023 at 11:11:30PM +0000, Imseih (AWS), Sami wrote: > I agree. We introduce the concept of a plannable statement in a > previous section and we can then make this distinction in the new > paragraph. > > I also added a link to pg_stat_statements_info since that is introduced > later on int the doc. I have reworded the paragraph a bit to be more general so as it would not need an update once more normalization is applied to utility queries (I am going to fix the part where we mention that we use the strings for utilities, which is not the case anymore now): + <para> + Queries on which normalization can be applied may be observed with constant + values in <structname>pg_stat_statements</structname>, especially when there + is a high rate of entry deallocations. To reduce the likelihood of this + happening, consider increasing <varname>pg_stat_statements.max</varname>. + The <structname>pg_stat_statements_info</structname> view, discussed below + in <xref linkend="pgstatstatements-pg-stat-statements-info"/>, + provides statistics about entry deallocations. + </para> Are you OK with this text? -- Michael Attachments: [text/x-diff] v3-0001-doc-update-regarding-pg_stat_statements-normaliza.patch (1.5K, ../../Y%[email protected]/2-v3-0001-doc-update-regarding-pg_stat_statements-normaliza.patch) download | inline diff: From dd8938e4ba1b1f29d14b3fa2dc76301f42592cad Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Wed, 1 Mar 2023 09:05:08 +0900 Subject: [PATCH v3] doc update regarding pg_stat_statements normalization. It is quite possible that a query text willl not normalize (replace constants with $1, $2..) when there is a high rate pgss_hash deallocation. This commit calls this out in docs. --- doc/src/sgml/pgstatstatements.sgml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml index efc36da602..f1ba78c8cb 100644 --- a/doc/src/sgml/pgstatstatements.sgml +++ b/doc/src/sgml/pgstatstatements.sgml @@ -516,6 +516,16 @@ <structname>pg_stat_statements</structname> entry. </para> + <para> + Queries on which normalization can be applied may be observed with constant + values in <structname>pg_stat_statements</structname>, especially when there + is a high rate of entry deallocations. To reduce the likelihood of this + happening, consider increasing <varname>pg_stat_statements.max</varname>. + The <structname>pg_stat_statements_info</structname> view, discussed below + in <xref linkend="pgstatstatements-pg-stat-statements-info"/>, + provides statistics about entry deallocations. + </para> + <para> In some cases, queries with visibly different texts might get merged into a single <structname>pg_stat_statements</structname> entry. Normally this will happen -- 2.39.2 [application/pgp-signature] signature.asc (833B, ../../Y%[email protected]/3-signature.asc) download ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-03-01 00:09 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-03-08 06:43 [PATCH 03/10] Make sure published XIDs are persistent Kyotaro Horiguchi <[email protected]> 2023-02-28 23:11 Re: Doc update for pg_stat_statements normalization Imseih (AWS), Sami <[email protected]> 2023-03-01 00:09 ` Re: Doc update for pg_stat_statements normalization Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox