public inbox for [email protected]  
help / color / mirror / Atom feed
From: Kirill Reshke <[email protected]>
To: Heikki Linnakangas <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: CSN snapshots in hot standby
Date: Fri, 5 Apr 2024 02:08:37 +0500
Message-ID: <CALdSSPgbnsq_ZOLd59tORg0-onQ0RVrMP_gQ_HudfJS14ecytQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>

Hi,

On Thu, 4 Apr 2024 at 22:21, Heikki Linnakangas <[email protected]> wrote:

> You cannot run queries on a Hot Standby server until the standby has
> seen a running-xacts record. Furthermore if the subxids cache had
> overflowed, you also need to wait for those transactions to finish. That
> is usually not a problem, because we write a running-xacts record after
> each checkpoint, and most systems don't use so many subtransactions that
> the cache would overflow. Still, you can run into it if you're unlucky,
> and it's annoying when you do.
>
> It occurred to me that we could replace the known-assigned-xids
> machinery with CSN snapshots. We've talked about CSN snapshots many
> times in the past, and I think it would make sense on the primary too,
> but for starters, we could use it just during Hot Standby.
>
> With CSN-based snapshots, you don't have the limitation with the
> fixed-size known-assigned-xids array, and overflowed sub-XIDs are not a
> problem either. You can always enter Hot Standby and start accepting
> queries as soon as the standby is in a physically consistent state.
>
> I dusted up and rebased the last CSN patch that I found on the mailing
> list [1], and modified it so that it's only used during recovery. That
> makes some things simpler and less scary. There are no changes to how
> transaction commit happens in the primary, the CSN log is only kept
> up-to-date in the standby, when commit/abort records are replayed. The
> CSN of each transaction is the LSN of its commit record.
>
> The CSN approach is much simpler than the existing known-assigned-XIDs
> machinery, as you can see from "git diff --stat" with this patch:
>
>   32 files changed, 773 insertions(+), 1711 deletions(-)
>
> With CSN snapshots, we don't need the known-assigned-XIDs machinery, and
> we can get rid of the xact-assignment records altogether. We no longer
> need the running-xacts records for Hot Standby either, but I wasn't able
> to remove that because it's still used by logical replication, in
> snapbuild.c. I have a feeling that that could somehow be simplified too,
> but didn't look into it.
>
> This is obviously v18 material, so I'll park this at the July commitfest
> for now. There are a bunch of little FIXMEs in the code, and needs
> performance testing, but overall I was surprised how easy this was.
>
> (We ran into this issue particularly hard with Neon, because with Neon
> you don't need to perform WAL replay at standby startup. However, when
> you don't perform WAL replay, you don't get to see the running-xact
> record after the checkpoint either. If the primary is idle, it doesn't
> generate new running-xact records, and the standby cannot start Hot
> Standby until the next time something happens in the primary. It's
> always a potential problem with overflowed sub-XIDs cache, but the lack
> of WAL replay made it happen even when there are no subtransactions
> involved.)
>
> [1]
> https://www.postgresql.org/message-id/2020081009525213277261%40highgo.ca
>
> --
> Heikki Linnakangas
> Neon (https://neon.tech)


Great. I really like the idea of vanishing KnownAssignedXids instead of
optimizing it (if optimizations are even possible).

> + /*
> + * TODO: We must mark CSNLOG first
> + */
> + CSNLogSetCSN(xid, parsed->nsubxacts, parsed->subxacts, lsn);
> +

As far as I understand we simply use the current Wal Record LSN as its XID
CSN number. Ok.
This seems to work for standbys snapshots, but this patch may be really
useful for distributed postgresql solutions, that use CSN for working
with distributed database snapshot (across multiple shards). These
solutions need to set CSN to some other value (time from True time/ClockSI
or whatever).
So, maybe we need some hooks here? Or maybe, we can take CSN here from
extension somehow. For example, we can define
some interface and extend it. Does this sound reasonable for you?

Also, I attached a patch which adds some more todos.


Attachments:

  [application/octet-stream] v1-0001-Point-comments-needed-to-be-edited.patch (2.3K, ../CALdSSPgbnsq_ZOLd59tORg0-onQ0RVrMP_gQ_HudfJS14ecytQ@mail.gmail.com/3-v1-0001-Point-comments-needed-to-be-edited.patch)
  download | inline diff:
From 4a361053b5947fa209fadd9d95cc9213e4052e31 Mon Sep 17 00:00:00 2001
From: reshke <[email protected]>
Date: Thu, 4 Apr 2024 20:50:54 +0000
Subject: [PATCH v1] Point comments needed to be edited

---
 contrib/pg_visibility/pg_visibility.c | 1 +
 src/backend/access/transam/xact.c     | 1 +
 src/backend/storage/ipc/standby.c     | 1 +
 3 files changed, 3 insertions(+)

diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c
index cb0c49c7a4..c5f46a3fc7 100644
--- a/contrib/pg_visibility/pg_visibility.c
+++ b/contrib/pg_visibility/pg_visibility.c
@@ -548,6 +548,7 @@ collect_visibility_data(Oid relid, bool include_pd)
  *    databases that were ignored before.
  * 2. Ignore KnownAssignedXids, because they are not database-aware. At the
  *    same time, the primary could compute its horizons database-aware.
+ * XXX: KnownAssignedXids is gone so the above comment needs updating.
  * 3. Ignore walsender xmin, because it could go backward if some replication
  *    connections don't use replication slots.
  *
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 5456954602..c81b9362e2 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1378,6 +1378,7 @@ RecordTransactionCommit(void)
 	 * KnownAssignedXids machinery requires tracking every XID assignment.  It
 	 * might be OK to skip it only when wal_level < replica, but for now we
 	 * don't.)
+	 * XXX: KnownAssignedXids is gone so the above comment needs updating.
 	 *
 	 * However, if we're doing cleanup of any non-temp rels or committing any
 	 * command that wanted to force sync commit, then we must flush XLOG
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 078ed53acd..a5d8a846cd 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -1267,6 +1267,7 @@ standby_redo(XLogReaderState *record)
  * an entry are expected and must not cause an error when we are in state
  * STANDBY_INITIALIZED. This is implemented in StandbyReleaseLocks() and
  * KnownAssignedXidsRemove().
+ * XXX: KnownAssignedXids is gone so the above comment needs updating.
  *
  * Later, when we apply the running xact data we must be careful to ignore
  * transactions already committed, since those commits raced ahead when
-- 
2.25.1



view thread (17+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: CSN snapshots in hot standby
  In-Reply-To: <CALdSSPgbnsq_ZOLd59tORg0-onQ0RVrMP_gQ_HudfJS14ecytQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox