public inbox for [email protected]help / color / mirror / Atom feed
[PATCH] psql: Show IP address in \conninfo, if different from host 44+ messages / 5 participants [nested] [flat]
* [PATCH] psql: Show IP address in \conninfo, if different from host @ 2018-11-19 15:23 Alvaro Herrera <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Alvaro Herrera @ 2018-11-19 15:23 UTC (permalink / raw) When hostaddr is given, the actual IP address that psql is connected to can be totally unexpected for the given host. The more verbose output we now generate makes things clearer. Since the "host" and "hostaddr" parts of the conninfo could come from wildly different sources (say one is in the service specification or a URI-style conninfo and the other is not), this is useful. This is also useful if a hostname resolves to multiple addresses. Author: Fabien Coelho Reviewed-by: Pavel Stehule, Arthur Zakirov Discussion: https://postgr.es/m/alpine.DEB.2.21.1810261532380.27686@lancre https://postgr.es/m/alpine.DEB.2.21.1808201323020.13832@lancre --- doc/src/sgml/libpq.sgml | 30 ++++++++++++ src/bin/psql/command.c | 82 +++++++++++++++++++++++++------- src/interfaces/libpq/exports.txt | 1 + src/interfaces/libpq/fe-connect.c | 98 +++++++++++++++++++++++++++++---------- src/interfaces/libpq/libpq-fe.h | 1 + src/interfaces/libpq/libpq-int.h | 1 + 6 files changed, 172 insertions(+), 41 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 601091c570..d2e5b08541 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1735,6 +1735,36 @@ char *PQhost(const PGconn *conn); </listitem> </varlistentry> + + <varlistentry id="libpq-pqhostaddr"> + <term> + <function>PQhostaddr</function> + <indexterm> + <primary>PQhostaddr</primary> + </indexterm> + </term> + + <listitem> + <para> + Returns the server IP address of the active connection. + This can be the address that a host name resolved to, + or an IP address provided through the <literal>hostaddr</literal> + parameter. +<synopsis> +char *PQhostaddr(const PGconn *conn); +</synopsis> + </para> + + <para> + <function>PQhostaddr</function> returns <symbol>NULL</symbol> if the + <parameter>conn</parameter> argument is <symbol>NULL</symbol>. + Otherwise, if there is an error producing the host information + (perhaps if the connection has not been fully established or + there was an error), it returns an empty string. + </para> + </listitem> + </varlistentry> + <varlistentry id="libpq-pqport"> <term> <function>PQport</function> diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 04e227b5a6..ee88e1ca5c 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -596,14 +596,30 @@ exec_command_conninfo(PsqlScanState scan_state, bool active_branch) else { char *host = PQhost(pset.db); + char *hostaddr = PQhostaddr(pset.db); - /* If the host is an absolute path, the connection is via socket */ + /* + * If the host is an absolute path, the connection is via socket + * unless overriden by hostaddr + */ if (is_absolute_path(host)) - printf(_("You are connected to database \"%s\" as user \"%s\" via socket in \"%s\" at port \"%s\".\n"), - db, PQuser(pset.db), host, PQport(pset.db)); + { + if (hostaddr && *hostaddr) + printf(_("You are connected to database \"%s\" as user \"%s\" on address \"%s\" at port \"%s\".\n"), + db, PQuser(pset.db), hostaddr, PQport(pset.db)); + else + printf(_("You are connected to database \"%s\" as user \"%s\" via socket in \"%s\" at port \"%s\".\n"), + db, PQuser(pset.db), host, PQport(pset.db)); + } else - printf(_("You are connected to database \"%s\" as user \"%s\" on host \"%s\" at port \"%s\".\n"), - db, PQuser(pset.db), host, PQport(pset.db)); + { + if (hostaddr && *hostaddr && strcmp(host, hostaddr) != 0) + printf(_("You are connected to database \"%s\" as user \"%s\" on host \"%s\" (address \"%s\") at port \"%s\".\n"), + db, PQuser(pset.db), host, hostaddr, PQport(pset.db)); + else + printf(_("You are connected to database \"%s\" as user \"%s\" on host \"%s\" at port \"%s\".\n"), + db, PQuser(pset.db), host, PQport(pset.db)); + } printSSLInfo(); } } @@ -2854,6 +2870,7 @@ do_connect(enum trivalue reuse_previous_specification, PGconn *o_conn = pset.db, *n_conn; char *password = NULL; + char *hostaddr = NULL; bool keep_password; bool has_connection_string; bool reuse_previous; @@ -2894,12 +2911,27 @@ do_connect(enum trivalue reuse_previous_specification, } /* grab missing values from the old connection */ - if (!user && reuse_previous) - user = PQuser(o_conn); - if (!host && reuse_previous) - host = PQhost(o_conn); - if (!port && reuse_previous) - port = PQport(o_conn); + if (reuse_previous) + { + if (!user) + user = PQuser(o_conn); + if (host && strcmp(host, PQhost(o_conn)) == 0) + { + /* + * if we are targetting the same host, reuse its hostaddr for + * consistency + */ + hostaddr = PQhostaddr(o_conn); + } + if (!host) + { + host = PQhost(o_conn); + /* also set hostaddr for consistency */ + hostaddr = PQhostaddr(o_conn); + } + if (!port) + port = PQport(o_conn); + } /* * Any change in the parameters read above makes us discard the password. @@ -2961,13 +2993,18 @@ do_connect(enum trivalue reuse_previous_specification, while (true) { -#define PARAMS_ARRAY_SIZE 8 +#define PARAMS_ARRAY_SIZE 9 const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords)); const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values)); int paramnum = -1; keywords[++paramnum] = "host"; values[paramnum] = host; + if (hostaddr && *hostaddr) + { + keywords[++paramnum] = "hostaddr"; + values[paramnum] = hostaddr; + } keywords[++paramnum] = "port"; values[paramnum] = port; keywords[++paramnum] = "user"; @@ -3071,14 +3108,27 @@ do_connect(enum trivalue reuse_previous_specification, param_is_newly_set(PQport(o_conn), PQport(pset.db))) { char *host = PQhost(pset.db); + char *hostaddr = PQhostaddr(pset.db); /* If the host is an absolute path, the connection is via socket */ if (is_absolute_path(host)) - printf(_("You are now connected to database \"%s\" as user \"%s\" via socket in \"%s\" at port \"%s\".\n"), - PQdb(pset.db), PQuser(pset.db), host, PQport(pset.db)); + { + if (hostaddr && *hostaddr) + printf(_("You are now connected to database \"%s\" as user \"%s\" on address \"%s\" at port \"%s\".\n"), + PQdb(pset.db), PQuser(pset.db), hostaddr, PQport(pset.db)); + else + printf(_("You are now connected to database \"%s\" as user \"%s\" via socket in \"%s\" at port \"%s\".\n"), + PQdb(pset.db), PQuser(pset.db), host, PQport(pset.db)); + } else - printf(_("You are now connected to database \"%s\" as user \"%s\" on host \"%s\" at port \"%s\".\n"), - PQdb(pset.db), PQuser(pset.db), host, PQport(pset.db)); + { + if (hostaddr && *hostaddr && strcmp(host, hostaddr) != 0) + printf(_("You are now connected to database \"%s\" as user \"%s\" on host \"%s\" (address \"%s\") at port \"%s\".\n"), + PQdb(pset.db), PQuser(pset.db), host, hostaddr, PQport(pset.db)); + else + printf(_("You are now connected to database \"%s\" as user \"%s\" on host \"%s\" at port \"%s\".\n"), + PQdb(pset.db), PQuser(pset.db), host, PQport(pset.db)); + } } else printf(_("You are now connected to database \"%s\" as user \"%s\".\n"), diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt index 4359fae30d..cc9ee9ce6b 100644 --- a/src/interfaces/libpq/exports.txt +++ b/src/interfaces/libpq/exports.txt @@ -173,3 +173,4 @@ PQsetErrorContextVisibility 170 PQresultVerboseErrorMessage 171 PQencryptPasswordConn 172 PQresultMemorySize 173 +PQhostaddr 174 diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index d001bc513d..bc456fec0c 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -1471,6 +1471,39 @@ connectNoDelay(PGconn *conn) return 1; } +/* ---------- + * Write currently connected IP address into host_addr (of len host_addr_len). + * If unable to, set it to the empty string. + * ---------- + */ +static void +getHostaddr(PGconn *conn, char *host_addr, int host_addr_len) +{ + struct sockaddr_storage *addr = &conn->raddr.addr; + + if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS) + strlcpy(host_addr, conn->connhost[conn->whichhost].hostaddr, host_addr_len); + else if (addr->ss_family == AF_INET) + { + if (inet_net_ntop(AF_INET, + &((struct sockaddr_in *) addr)->sin_addr.s_addr, + 32, + host_addr, host_addr_len) == NULL) + host_addr[0] = '\0'; + } +#ifdef HAVE_IPV6 + else if (addr->ss_family == AF_INET6) + { + if (inet_net_ntop(AF_INET6, + &((struct sockaddr_in6 *) addr)->sin6_addr.s6_addr, + 128, + host_addr, host_addr_len) == NULL) + host_addr[0] = '\0'; + } +#endif + else + host_addr[0] = '\0'; +} /* ---------- * connectFailureMessage - @@ -1504,34 +1537,12 @@ connectFailureMessage(PGconn *conn, int errorno) char host_addr[NI_MAXHOST]; const char *displayed_host; const char *displayed_port; - struct sockaddr_storage *addr = &conn->raddr.addr; /* * Optionally display the network address with the hostname. This is * useful to distinguish between IPv4 and IPv6 connections. */ - if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS) - strlcpy(host_addr, conn->connhost[conn->whichhost].hostaddr, NI_MAXHOST); - else if (addr->ss_family == AF_INET) - { - if (inet_net_ntop(AF_INET, - &((struct sockaddr_in *) addr)->sin_addr.s_addr, - 32, - host_addr, sizeof(host_addr)) == NULL) - strcpy(host_addr, "???"); - } -#ifdef HAVE_IPV6 - else if (addr->ss_family == AF_INET6) - { - if (inet_net_ntop(AF_INET6, - &((struct sockaddr_in6 *) addr)->sin6_addr.s6_addr, - 128, - host_addr, sizeof(host_addr)) == NULL) - strcpy(host_addr, "???"); - } -#endif - else - strcpy(host_addr, "???"); + getHostaddr(conn, host_addr, NI_MAXHOST); /* To which host and port were we actually connecting? */ if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS) @@ -1548,14 +1559,14 @@ connectFailureMessage(PGconn *conn, int errorno) * looked-up IP address. */ if (conn->connhost[conn->whichhost].type != CHT_HOST_ADDRESS && + strlen(host_addr) > 0 && strcmp(displayed_host, host_addr) != 0) appendPQExpBuffer(&conn->errorMessage, libpq_gettext("could not connect to server: %s\n" "\tIs the server running on host \"%s\" (%s) and accepting\n" "\tTCP/IP connections on port %s?\n"), SOCK_STRERROR(errorno, sebuf, sizeof(sebuf)), - displayed_host, - host_addr, + displayed_host, host_addr, displayed_port); else appendPQExpBuffer(&conn->errorMessage, @@ -2286,6 +2297,7 @@ keep_going: /* We will come back to here until there is */ { struct addrinfo *addr_cur = conn->addr_cur; + char host_addr[NI_MAXHOST]; /* * Advance to next possible host, if we've tried all of @@ -2302,6 +2314,21 @@ keep_going: /* We will come back to here until there is addr_cur->ai_addrlen); conn->raddr.salen = addr_cur->ai_addrlen; + /* set connip */ + if (conn->connip != NULL) + { + free(conn->connip); + conn->connip = NULL; + } + + getHostaddr(conn, host_addr, NI_MAXHOST); + if (strlen(host_addr) > 0) + conn->connip = strdup(host_addr); + /* + * purposely ignore strdup failure; not a big problem if + * it fails anyway. + */ + conn->sock = socket(addr_cur->ai_family, SOCK_STREAM, 0); if (conn->sock == PGINVALID_SOCKET) { @@ -3665,6 +3692,8 @@ freePGconn(PGconn *conn) free(conn->sslcompression); if (conn->requirepeer) free(conn->requirepeer); + if (conn->connip) + free(conn->connip); #if defined(ENABLE_GSS) || defined(ENABLE_SSPI) if (conn->krbsrvname) free(conn->krbsrvname); @@ -6173,6 +6202,25 @@ PQhost(const PGconn *conn) } char * +PQhostaddr(const PGconn *conn) +{ + if (!conn) + return NULL; + + if (conn->connhost != NULL) + { + if (conn->connhost[conn->whichhost].hostaddr != NULL && + conn->connhost[conn->whichhost].hostaddr[0] != '\0') + return conn->connhost[conn->whichhost].hostaddr; + + if (conn->connip != NULL) + return conn->connip; + } + + return ""; +} + +char * PQport(const PGconn *conn) { if (!conn) diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h index 52bd5d2cd8..3f13ddf092 100644 --- a/src/interfaces/libpq/libpq-fe.h +++ b/src/interfaces/libpq/libpq-fe.h @@ -312,6 +312,7 @@ extern char *PQdb(const PGconn *conn); extern char *PQuser(const PGconn *conn); extern char *PQpass(const PGconn *conn); extern char *PQhost(const PGconn *conn); +extern char *PQhostaddr(const PGconn *conn); extern char *PQport(const PGconn *conn); extern char *PQtty(const PGconn *conn); extern char *PQoptions(const PGconn *conn); diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 975ab33d02..66fd317b94 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -397,6 +397,7 @@ struct pg_conn int nconnhost; /* # of hosts named in conn string */ int whichhost; /* host we're currently trying/connected to */ pg_conn_host *connhost; /* details about each named host */ + char *connip; /* IP address for current network connection */ /* Connection data */ pgsocket sock; /* FD for socket, PGINVALID_SOCKET if -- 2.11.0 --fglnnjmzy32sjv22-- ^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet @ 2023-01-18 21:14 Nathan Bossart <[email protected]> 0 siblings, 2 replies; 44+ messages in thread From: Nathan Bossart @ 2023-01-18 21:14 UTC (permalink / raw) To: Robert Haas <[email protected]>; +Cc: pgsql-hackers On Wed, Jan 18, 2023 at 02:51:38PM -0500, Robert Haas wrote: > Should (nfree < SuperuserReservedBackends) be using <=, or am I confused? I believe < is correct. At this point, the new backend will have already claimed a proc struct, so if the number of remaining free slots equals the number of reserved slots, it is okay. > What's the deal with removing "and no new replication connections will > be accepted" from the documentation? Is the existing documentation > just wrong? If so, should we fix that first? And maybe delete > "non-replication" from the error message that says "remaining > connection slots are reserved for non-replication superuser > connections"? It seems like right now the comments say that > replication connections are a completely separate pool of connections, > but the documentation and the error message make it sound otherwise. > If that's true, then one of them is wrong, and I think it's the > docs/error message. Or am I just misreading it? I think you are right. This seems to have been missed in ea92368. I moved this part to a new patch that should probably be back-patched to v12. On that note, I wonder if it's worth changing the "sorry, too many clients already" message to make it clear that max_connections has been reached. IME some users are confused by this error, and I think it would be less confusing if it pointed to the parameter that governs the number of connection slots. I'll create a new thread for this. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com Attachments: [text/x-diff] v3-0001-Code-review-for-ea92368.patch (1.6K, ../../20230118211414.GA3702050@nathanxps13/2-v3-0001-Code-review-for-ea92368.patch) download | inline diff: From 8f0aa2fa54ae01149cffe9a69265f98e76a08a23 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Wed, 18 Jan 2023 12:43:41 -0800 Subject: [PATCH v3 1/3] Code review for ea92368. This commit missed an error message and a line in the docs. Back-patch to v12. --- doc/src/sgml/config.sgml | 3 +-- src/backend/utils/init/postinit.c | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 89d53f2a64..e019a1aac9 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -725,8 +725,7 @@ include_dir 'conf.d' number of active concurrent connections is at least <varname>max_connections</varname> minus <varname>superuser_reserved_connections</varname>, new - connections will be accepted only for superusers, and no - new replication connections will be accepted. + connections will be accepted only for superusers. </para> <para> diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index ae5a85ed65..9145d96b38 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -931,7 +931,7 @@ InitPostgres(const char *in_dbname, Oid dboid, !HaveNFreeProcs(ReservedBackends)) ereport(FATAL, (errcode(ERRCODE_TOO_MANY_CONNECTIONS), - errmsg("remaining connection slots are reserved for non-replication superuser connections"))); + errmsg("remaining connection slots are reserved for superusers"))); /* Check replication permissions needed for walsender processes. */ if (am_walsender) -- 2.25.1 [text/x-diff] v3-0002-Rename-ReservedBackends-to-SuperuserReservedBacke.patch (4.0K, ../../20230118211414.GA3702050@nathanxps13/3-v3-0002-Rename-ReservedBackends-to-SuperuserReservedBacke.patch) download | inline diff: From bc110461e4f0a73c2b76ba0a6e821349c2cbe3df Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Tue, 17 Jan 2023 13:58:56 -0800 Subject: [PATCH v3 2/3] Rename ReservedBackends to SuperuserReservedBackends. This is in preparation for adding a new reserved_connections GUC that will use the ReservedBackends variable name. --- src/backend/postmaster/postmaster.c | 18 +++++++++--------- src/backend/utils/init/postinit.c | 4 ++-- src/backend/utils/misc/guc_tables.c | 2 +- src/include/postmaster/postmaster.h | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 9cedc1b9f0..470704f364 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -204,15 +204,15 @@ char *Unix_socket_directories; char *ListenAddresses; /* - * ReservedBackends is the number of backends reserved for superuser use. - * This number is taken out of the pool size given by MaxConnections so + * SuperuserReservedBackends is the number of backends reserved for superuser + * use. This number is taken out of the pool size given by MaxConnections so * number of backend slots available to non-superusers is - * (MaxConnections - ReservedBackends). Note what this really means is - * "if there are <= ReservedBackends connections available, only superusers - * can make new connections" --- pre-existing superuser connections don't - * count against the limit. + * (MaxConnections - SuperuserReservedBackends). Note what this really means + * is "if there are <= SuperuserReservedBackends connections available, only + * superusers can make new connections" --- pre-existing superuser connections + * don't count against the limit. */ -int ReservedBackends; +int SuperuserReservedBackends; /* The socket(s) we're listening to. */ #define MAXLISTEN 64 @@ -908,11 +908,11 @@ PostmasterMain(int argc, char *argv[]) /* * Check for invalid combinations of GUC settings. */ - if (ReservedBackends >= MaxConnections) + if (SuperuserReservedBackends >= MaxConnections) { write_stderr("%s: superuser_reserved_connections (%d) must be less than max_connections (%d)\n", progname, - ReservedBackends, MaxConnections); + SuperuserReservedBackends, MaxConnections); ExitPostmaster(1); } if (XLogArchiveMode > ARCHIVE_MODE_OFF && wal_level == WAL_LEVEL_MINIMAL) diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 9145d96b38..531a80f8d5 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -927,8 +927,8 @@ InitPostgres(const char *in_dbname, Oid dboid, * limited by max_connections or superuser_reserved_connections. */ if (!am_superuser && !am_walsender && - ReservedBackends > 0 && - !HaveNFreeProcs(ReservedBackends)) + SuperuserReservedBackends > 0 && + !HaveNFreeProcs(SuperuserReservedBackends)) ereport(FATAL, (errcode(ERRCODE_TOO_MANY_CONNECTIONS), errmsg("remaining connection slots are reserved for superusers"))); diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 5025e80f89..5aa2cda8f9 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -2163,7 +2163,7 @@ struct config_int ConfigureNamesInt[] = gettext_noop("Sets the number of connection slots reserved for superusers."), NULL }, - &ReservedBackends, + &SuperuserReservedBackends, 3, 0, MAX_BACKENDS, NULL, NULL, NULL }, diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h index 203177e1ff..168d85a3d1 100644 --- a/src/include/postmaster/postmaster.h +++ b/src/include/postmaster/postmaster.h @@ -15,7 +15,7 @@ /* GUC options */ extern PGDLLIMPORT bool EnableSSL; -extern PGDLLIMPORT int ReservedBackends; +extern PGDLLIMPORT int SuperuserReservedBackends; extern PGDLLIMPORT int PostPortNumber; extern PGDLLIMPORT int Unix_socket_permissions; extern PGDLLIMPORT char *Unix_socket_group; -- 2.25.1 [text/x-diff] v3-0003-Introduce-reserved_connections-and-pg_use_reserve.patch (12.8K, ../../20230118211414.GA3702050@nathanxps13/4-v3-0003-Introduce-reserved_connections-and-pg_use_reserve.patch) download | inline diff: From 17bd6189138c49bc222614195cbf202274ea8e9b Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Tue, 17 Jan 2023 15:36:59 -0800 Subject: [PATCH v3 3/3] Introduce reserved_connections and pg_use_reserved_connections. This provides a way to reserve connection slots for non-superusers. superuser_reserved_connections remains as a final reserve in case reserved_connections has been exhausted. --- doc/src/sgml/config.sgml | 39 ++++++++++++++++++- doc/src/sgml/user-manag.sgml | 5 +++ src/backend/postmaster/postmaster.c | 28 ++++++++----- src/backend/storage/lmgr/proc.c | 16 +++++--- src/backend/utils/init/postinit.c | 27 +++++++++---- src/backend/utils/misc/guc_tables.c | 11 ++++++ src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/catalog/pg_authid.dat | 5 +++ src/include/postmaster/postmaster.h | 1 + src/include/storage/proc.h | 2 +- 10 files changed, 110 insertions(+), 25 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index e019a1aac9..626faa4d08 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -725,17 +725,52 @@ include_dir 'conf.d' number of active concurrent connections is at least <varname>max_connections</varname> minus <varname>superuser_reserved_connections</varname>, new - connections will be accepted only for superusers. + connections will be accepted only for superusers. The connection slots + reserved by this parameter are intended as final reserve for emergency + use after the slots reserved by + <xref linkend="guc-reserved-connections"/> have been exhausted. </para> <para> The default value is three connections. The value must be less - than <varname>max_connections</varname>. + than <varname>max_connections</varname> minus + <varname>reserved_connections</varname>. This parameter can only be set at server start. </para> </listitem> </varlistentry> + <varlistentry id="guc-reserved-connections" xreflabel="reserved_connections"> + <term><varname>reserved_connections</varname> (<type>integer</type>) + <indexterm> + <primary><varname>reserved_connections</varname> configuration parameter</primary> + </indexterm> + </term> + <listitem> + <para> + Determines the number of connection <quote>slots</quote> that are + reserved for connections by roles with privileges of the + <link linkend="predefined-roles-table"><literal>pg_used_reserved_connections</literal></link> + role. Whenever the number of free connection slots is greater than + <xref linkend="guc-superuser-reserved-connections"/> but less than or + equal to the sum of <varname>superuser_reserved_connections</varname> + and <varname>reserved_connections</varname>, new connections will be + accepted only for superusers and roles with privileges of + <literal>pg_use_reserved_connections</literal>. If + <varname>superuser_reserved_connections</varname> or fewer connection + slots are available, new connections will be accepted only for + superusers. + </para> + + <para> + The default value is zero connections. The value must be less than + <varname>max_connections</varname> minus + <varname>superuser_reserved_connections</varname>. This parameter can + only be set at server start. + </para> + </listitem> + </varlistentry> + <varlistentry id="guc-unix-socket-directories" xreflabel="unix_socket_directories"> <term><varname>unix_socket_directories</varname> (<type>string</type>) <indexterm> diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml index 71a2d8f298..d553b1adab 100644 --- a/doc/src/sgml/user-manag.sgml +++ b/doc/src/sgml/user-manag.sgml @@ -689,6 +689,11 @@ DROP ROLE doomed_role; and <link linkend="sql-lock"><command>LOCK TABLE</command></link> on all relations.</entry> </row> + <row> + <entry>pg_use_reserved_backends</entry> + <entry>Allow use of connection slots reserved via + <xref linkend="guc-reserved-connections"/>.</entry> + </row> </tbody> </tgroup> </table> diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 470704f364..ac69d3fd9b 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -205,14 +205,23 @@ char *ListenAddresses; /* * SuperuserReservedBackends is the number of backends reserved for superuser - * use. This number is taken out of the pool size given by MaxConnections so - * number of backend slots available to non-superusers is - * (MaxConnections - SuperuserReservedBackends). Note what this really means - * is "if there are <= SuperuserReservedBackends connections available, only - * superusers can make new connections" --- pre-existing superuser connections - * don't count against the limit. + * use, and ReservedBackends is the number of backends reserved for use by + * roles with privileges of the pg_use_reserved_backends predefined role. + * These are taken out of the pool of MaxConnections backend slots, so the + * number of backend slots available for roles that are neither superuser nor + * have privileges of pg_use_reserved_backends is + * (MaxConnections - SuperuserReservedBackends - ReservedBackends). + * + * If the number of remaining slots is less than or equal to + * SuperuserReservedBackends, only superusers can make new connections. If the + * number of remaining slots is greater than SuperuserReservedBackends but less + * than or equal to (SuperuserReservedBackends + ReservedBackends), only + * superusers and roles with privileges of pg_use_reserved_backends can make + * new connections. Note that pre-existing superuser and + * pg_use_reserved_backends connections don't count against the limits. */ int SuperuserReservedBackends; +int ReservedBackends; /* The socket(s) we're listening to. */ #define MAXLISTEN 64 @@ -908,11 +917,12 @@ PostmasterMain(int argc, char *argv[]) /* * Check for invalid combinations of GUC settings. */ - if (SuperuserReservedBackends >= MaxConnections) + if (SuperuserReservedBackends + ReservedBackends >= MaxConnections) { - write_stderr("%s: superuser_reserved_connections (%d) must be less than max_connections (%d)\n", + write_stderr("%s: superuser_reserved_connections (%d) plus reserved_connections (%d) must be less than max_connections (%d)\n", progname, - SuperuserReservedBackends, MaxConnections); + SuperuserReservedBackends, ReservedBackends, + MaxConnections); ExitPostmaster(1); } if (XLogArchiveMode > ARCHIVE_MODE_OFF && wal_level == WAL_LEVEL_MINIMAL) diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index f8ac4edd6f..22b4278610 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -645,27 +645,33 @@ GetStartupBufferPinWaitBufId(void) } /* - * Check whether there are at least N free PGPROC objects. + * Check whether there are at least N free PGPROC objects. If false is + * returned, *nfree will be set to the number of free PGPROC objects. + * Otherwise, *nfree will be set to n. * * Note: this is designed on the assumption that N will generally be small. */ bool -HaveNFreeProcs(int n) +HaveNFreeProcs(int n, int *nfree) { dlist_iter iter; + Assert(n > 0); + Assert(nfree); + SpinLockAcquire(ProcStructLock); + *nfree = 0; dlist_foreach(iter, &ProcGlobal->freeProcs) { - n--; - if (n == 0) + (*nfree)++; + if (*nfree == n) break; } SpinLockRelease(ProcStructLock); - return (n <= 0); + return (*nfree == n); } /* diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 531a80f8d5..bd7ece259a 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -719,6 +719,7 @@ InitPostgres(const char *in_dbname, Oid dboid, bool am_superuser; char *fullpath; char dbname[NAMEDATALEN]; + int nfree = 0; elog(DEBUG3, "InitPostgres"); @@ -922,16 +923,26 @@ InitPostgres(const char *in_dbname, Oid dboid, } /* - * The last few connection slots are reserved for superusers. Replication - * connections are drawn from slots reserved with max_wal_senders and not - * limited by max_connections or superuser_reserved_connections. + * The last few connection slots are reserved for superusers and roles with + * privileges of pg_use_reserved_connections. Replication connections are + * drawn from slots reserved with max_wal_senders and are not limited by + * max_connections, superuser_reserved_connections, or + * reserved_connections. */ if (!am_superuser && !am_walsender && - SuperuserReservedBackends > 0 && - !HaveNFreeProcs(SuperuserReservedBackends)) - ereport(FATAL, - (errcode(ERRCODE_TOO_MANY_CONNECTIONS), - errmsg("remaining connection slots are reserved for superusers"))); + (SuperuserReservedBackends + ReservedBackends) > 0 && + !HaveNFreeProcs(SuperuserReservedBackends + ReservedBackends, &nfree)) + { + if (nfree < SuperuserReservedBackends) + ereport(FATAL, + (errcode(ERRCODE_TOO_MANY_CONNECTIONS), + errmsg("remaining connection slots are reserved for superusers"))); + + if (!has_privs_of_role(GetUserId(), ROLE_PG_USE_RESERVED_CONNECTIONS)) + ereport(FATAL, + (errcode(ERRCODE_TOO_MANY_CONNECTIONS), + errmsg("remaining connection slots are reserved for roles with privileges of pg_use_reserved_backends"))); + } /* Check replication permissions needed for walsender processes. */ if (am_walsender) diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 5aa2cda8f9..77b17aa7a6 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -2168,6 +2168,17 @@ struct config_int ConfigureNamesInt[] = NULL, NULL, NULL }, + { + {"reserved_connections", PGC_POSTMASTER, CONN_AUTH_SETTINGS, + gettext_noop("Sets the number of connection slots reserved for roles " + "with privileges of pg_use_reserved_connections."), + NULL + }, + &ReservedBackends, + 0, 0, MAX_BACKENDS, + NULL, NULL, NULL + }, + { {"min_dynamic_shared_memory", PGC_POSTMASTER, RESOURCES_MEM, gettext_noop("Amount of dynamic shared memory reserved at startup."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 4cceda4162..f701312225 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -64,6 +64,7 @@ #port = 5432 # (change requires restart) #max_connections = 100 # (change requires restart) #superuser_reserved_connections = 3 # (change requires restart) +#reserved_connections = 0 # (change requires restart) #unix_socket_directories = '/tmp' # comma-separated list of directories # (change requires restart) #unix_socket_group = '' # (change requires restart) diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat index 2a2fee7d28..f2e5663c9f 100644 --- a/src/include/catalog/pg_authid.dat +++ b/src/include/catalog/pg_authid.dat @@ -89,5 +89,10 @@ rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', rolpassword => '_null_', rolvaliduntil => '_null_' }, +{ oid => '4550', oid_symbol => 'ROLE_PG_USE_RESERVED_CONNECTIONS', + rolname => 'pg_use_reserved_connections', rolsuper => 'f', rolinherit => 't', + rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', + rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', + rolpassword => '_null_', rolvaliduntil => '_null_' }, ] diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h index 168d85a3d1..18ef5afa75 100644 --- a/src/include/postmaster/postmaster.h +++ b/src/include/postmaster/postmaster.h @@ -16,6 +16,7 @@ /* GUC options */ extern PGDLLIMPORT bool EnableSSL; extern PGDLLIMPORT int SuperuserReservedBackends; +extern PGDLLIMPORT int ReservedBackends; extern PGDLLIMPORT int PostPortNumber; extern PGDLLIMPORT int Unix_socket_permissions; extern PGDLLIMPORT char *Unix_socket_group; diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index dd45b8ee9b..4258cd92c9 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -445,7 +445,7 @@ extern void InitAuxiliaryProcess(void); extern void SetStartupBufferPinWaitBufId(int bufid); extern int GetStartupBufferPinWaitBufId(void); -extern bool HaveNFreeProcs(int n); +extern bool HaveNFreeProcs(int n, int *nfree); extern void ProcReleaseLocks(bool isCommit); extern ProcWaitStatus ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable); -- 2.25.1 ^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet @ 2023-01-19 12:58 tushar <[email protected]> parent: Nathan Bossart <[email protected]> 1 sibling, 2 replies; 44+ messages in thread From: tushar @ 2023-01-19 12:58 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; Robert Haas <[email protected]>; +Cc: pgsql-hackers On 1/19/23 2:44 AM, Nathan Bossart wrote: > On Wed, Jan 18, 2023 at 02:51:38PM -0500, Robert Haas wrote: >> Should (nfree < SuperuserReservedBackends) be using <=, or am I confused? > I believe < is correct. At this point, the new backend will have already > claimed a proc struct, so if the number of remaining free slots equals the > number of reserved slots, it is okay. > >> What's the deal with removing "and no new replication connections will >> be accepted" from the documentation? Is the existing documentation >> just wrong? If so, should we fix that first? And maybe delete >> "non-replication" from the error message that says "remaining >> connection slots are reserved for non-replication superuser >> connections"? It seems like right now the comments say that >> replication connections are a completely separate pool of connections, >> but the documentation and the error message make it sound otherwise. >> If that's true, then one of them is wrong, and I think it's the >> docs/error message. Or am I just misreading it? > I think you are right. This seems to have been missed in ea92368. I moved > this part to a new patch that should probably be back-patched to v12. > > On that note, I wonder if it's worth changing the "sorry, too many clients > already" message to make it clear that max_connections has been reached. > IME some users are confused by this error, and I think it would be less > confusing if it pointed to the parameter that governs the number of > connection slots. I'll create a new thread for this. > There is one typo , for the doc changes, it is mentioned "pg_use_reserved_backends" but i think it supposed to be "pg_use_reserved_connections" under Table 22.1. Predefined Roles. -- regards,tushar EnterpriseDB https://www.enterprisedb.com/ The Enterprise PostgreSQL Company ^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet @ 2023-01-19 13:20 tushar <[email protected]> parent: tushar <[email protected]> 1 sibling, 0 replies; 44+ messages in thread From: tushar @ 2023-01-19 13:20 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; Robert Haas <[email protected]>; +Cc: pgsql-hackers On Thu, Jan 19, 2023 at 6:28 PM tushar <[email protected]> wrote: > On 1/19/23 2:44 AM, Nathan Bossart wrote: > > On Wed, Jan 18, 2023 at 02:51:38PM -0500, Robert Haas wrote: > >> Should (nfree < SuperuserReservedBackends) be using <=, or am I > confused? > > I believe < is correct. At this point, the new backend will have already > > claimed a proc struct, so if the number of remaining free slots equals > the > > number of reserved slots, it is okay. > > > >> What's the deal with removing "and no new replication connections will > >> be accepted" from the documentation? Is the existing documentation > >> just wrong? If so, should we fix that first? And maybe delete > >> "non-replication" from the error message that says "remaining > >> connection slots are reserved for non-replication superuser > >> connections"? It seems like right now the comments say that > >> replication connections are a completely separate pool of connections, > >> but the documentation and the error message make it sound otherwise. > >> If that's true, then one of them is wrong, and I think it's the > >> docs/error message. Or am I just misreading it? > > I think you are right. This seems to have been missed in ea92368. I > moved > > this part to a new patch that should probably be back-patched to v12. > > > > On that note, I wonder if it's worth changing the "sorry, too many > clients > > already" message to make it clear that max_connections has been reached. > > IME some users are confused by this error, and I think it would be less > > confusing if it pointed to the parameter that governs the number of > > connection slots. I'll create a new thread for this. > > > There is one typo , for the doc changes, it is mentioned > "pg_use_reserved_backends" but i think it supposed to be > "pg_use_reserved_connections" > under Table 22.1. Predefined Roles. > > and in the error message too [edb@centos7tushar bin]$ ./psql postgres -U r2 psql: error: connection to server on socket "/tmp/.s.PGSQL.5432" failed: FATAL: remaining connection slots are reserved for roles with privileges of pg_use_reserved_backends [edb@centos7tushar bin]$ regards, ^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet @ 2023-01-19 16:40 Robert Haas <[email protected]> parent: Nathan Bossart <[email protected]> 1 sibling, 1 reply; 44+ messages in thread From: Robert Haas @ 2023-01-19 16:40 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: pgsql-hackers On Wed, Jan 18, 2023 at 4:14 PM Nathan Bossart <[email protected]> wrote: > On Wed, Jan 18, 2023 at 02:51:38PM -0500, Robert Haas wrote: > > Should (nfree < SuperuserReservedBackends) be using <=, or am I confused? > > I believe < is correct. At this point, the new backend will have already > claimed a proc struct, so if the number of remaining free slots equals the > number of reserved slots, it is okay. OK. Might be worth a short comment. > > What's the deal with removing "and no new replication connections will > > be accepted" from the documentation? Is the existing documentation > > just wrong? If so, should we fix that first? And maybe delete > > "non-replication" from the error message that says "remaining > > connection slots are reserved for non-replication superuser > > connections"? It seems like right now the comments say that > > replication connections are a completely separate pool of connections, > > but the documentation and the error message make it sound otherwise. > > If that's true, then one of them is wrong, and I think it's the > > docs/error message. Or am I just misreading it? > > I think you are right. This seems to have been missed in ea92368. I moved > this part to a new patch that should probably be back-patched to v12. I'm inclined to commit it to master and not back-patch. It doesn't seem important enough to perturb translations. Tushar seems to have a point about pg_use_reserved_connections vs. pg_use_reserved_backends. I think we should standardize on the former, as backends is an internal term. -- Robert Haas EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet @ 2023-01-19 17:54 Nathan Bossart <[email protected]> parent: Robert Haas <[email protected]> 0 siblings, 1 reply; 44+ messages in thread From: Nathan Bossart @ 2023-01-19 17:54 UTC (permalink / raw) To: Robert Haas <[email protected]>; +Cc: pgsql-hackers On Thu, Jan 19, 2023 at 11:40:53AM -0500, Robert Haas wrote: > On Wed, Jan 18, 2023 at 4:14 PM Nathan Bossart <[email protected]> wrote: >> On Wed, Jan 18, 2023 at 02:51:38PM -0500, Robert Haas wrote: >> > Should (nfree < SuperuserReservedBackends) be using <=, or am I confused? >> >> I believe < is correct. At this point, the new backend will have already >> claimed a proc struct, so if the number of remaining free slots equals the >> number of reserved slots, it is okay. > > OK. Might be worth a short comment. I added one. >> > What's the deal with removing "and no new replication connections will >> > be accepted" from the documentation? Is the existing documentation >> > just wrong? If so, should we fix that first? And maybe delete >> > "non-replication" from the error message that says "remaining >> > connection slots are reserved for non-replication superuser >> > connections"? It seems like right now the comments say that >> > replication connections are a completely separate pool of connections, >> > but the documentation and the error message make it sound otherwise. >> > If that's true, then one of them is wrong, and I think it's the >> > docs/error message. Or am I just misreading it? >> >> I think you are right. This seems to have been missed in ea92368. I moved >> this part to a new patch that should probably be back-patched to v12. > > I'm inclined to commit it to master and not back-patch. It doesn't > seem important enough to perturb translations. That seems reasonable to me. > Tushar seems to have a point about pg_use_reserved_connections vs. > pg_use_reserved_backends. I think we should standardize on the former, > as backends is an internal term. Oops. This is what I meant to do. I probably flubbed it because I was wondering why the parameter uses "connections" and the variable uses "backends," especially considering that the variable for max_connections is called MaxConnections. I went ahead and renamed everything to use "connections." -- Nathan Bossart Amazon Web Services: https://aws.amazon.com Attachments: [text/x-diff] v4-0001-Code-review-for-ea92368.patch (1.6K, ../../20230119175421.GA3934756@nathanxps13/2-v4-0001-Code-review-for-ea92368.patch) download | inline diff: From 7be7e70aaf488a924d61b21b351a3b4f7e33aedc Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Wed, 18 Jan 2023 12:43:41 -0800 Subject: [PATCH v4 1/3] Code review for ea92368. This commit missed an error message and a line in the docs. --- doc/src/sgml/config.sgml | 3 +-- src/backend/utils/init/postinit.c | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 89d53f2a64..e019a1aac9 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -725,8 +725,7 @@ include_dir 'conf.d' number of active concurrent connections is at least <varname>max_connections</varname> minus <varname>superuser_reserved_connections</varname>, new - connections will be accepted only for superusers, and no - new replication connections will be accepted. + connections will be accepted only for superusers. </para> <para> diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index ae5a85ed65..9145d96b38 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -931,7 +931,7 @@ InitPostgres(const char *in_dbname, Oid dboid, !HaveNFreeProcs(ReservedBackends)) ereport(FATAL, (errcode(ERRCODE_TOO_MANY_CONNECTIONS), - errmsg("remaining connection slots are reserved for non-replication superuser connections"))); + errmsg("remaining connection slots are reserved for superusers"))); /* Check replication permissions needed for walsender processes. */ if (am_walsender) -- 2.25.1 [text/x-diff] v4-0002-Rename-ReservedBackends-to-SuperuserReservedConne.patch (4.1K, ../../20230119175421.GA3934756@nathanxps13/3-v4-0002-Rename-ReservedBackends-to-SuperuserReservedConne.patch) download | inline diff: From 058df2b3dcf50ecbe76c794f4f52751e6a9f765f Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Tue, 17 Jan 2023 13:58:56 -0800 Subject: [PATCH v4 2/3] Rename ReservedBackends to SuperuserReservedConnections. This is in preparation for adding a new reserved_connections GUC. --- src/backend/postmaster/postmaster.c | 20 ++++++++++---------- src/backend/utils/init/postinit.c | 4 ++-- src/backend/utils/misc/guc_tables.c | 2 +- src/include/postmaster/postmaster.h | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 9cedc1b9f0..3f799c4ac8 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -204,15 +204,15 @@ char *Unix_socket_directories; char *ListenAddresses; /* - * ReservedBackends is the number of backends reserved for superuser use. - * This number is taken out of the pool size given by MaxConnections so - * number of backend slots available to non-superusers is - * (MaxConnections - ReservedBackends). Note what this really means is - * "if there are <= ReservedBackends connections available, only superusers - * can make new connections" --- pre-existing superuser connections don't - * count against the limit. + * SuperuserReservedConnections is the number of backends reserved for + * superuser use. This number is taken out of the pool size given by + * MaxConnections so number of backend slots available to non-superusers is + * (MaxConnections - SuperuserReservedConnections). Note what this really + * means is "if there are <= SuperuserReservedConnections connections + * available, only superusers can make new connections" --- pre-existing + * superuser connections don't count against the limit. */ -int ReservedBackends; +int SuperuserReservedConnections; /* The socket(s) we're listening to. */ #define MAXLISTEN 64 @@ -908,11 +908,11 @@ PostmasterMain(int argc, char *argv[]) /* * Check for invalid combinations of GUC settings. */ - if (ReservedBackends >= MaxConnections) + if (SuperuserReservedConnections >= MaxConnections) { write_stderr("%s: superuser_reserved_connections (%d) must be less than max_connections (%d)\n", progname, - ReservedBackends, MaxConnections); + SuperuserReservedConnections, MaxConnections); ExitPostmaster(1); } if (XLogArchiveMode > ARCHIVE_MODE_OFF && wal_level == WAL_LEVEL_MINIMAL) diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 9145d96b38..40f145e0ab 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -927,8 +927,8 @@ InitPostgres(const char *in_dbname, Oid dboid, * limited by max_connections or superuser_reserved_connections. */ if (!am_superuser && !am_walsender && - ReservedBackends > 0 && - !HaveNFreeProcs(ReservedBackends)) + SuperuserReservedConnections > 0 && + !HaveNFreeProcs(SuperuserReservedConnections)) ereport(FATAL, (errcode(ERRCODE_TOO_MANY_CONNECTIONS), errmsg("remaining connection slots are reserved for superusers"))); diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index cd0fc2cb8f..0fa9fdd3c5 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -2163,7 +2163,7 @@ struct config_int ConfigureNamesInt[] = gettext_noop("Sets the number of connection slots reserved for superusers."), NULL }, - &ReservedBackends, + &SuperuserReservedConnections, 3, 0, MAX_BACKENDS, NULL, NULL, NULL }, diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h index 203177e1ff..0e4b8ded34 100644 --- a/src/include/postmaster/postmaster.h +++ b/src/include/postmaster/postmaster.h @@ -15,7 +15,7 @@ /* GUC options */ extern PGDLLIMPORT bool EnableSSL; -extern PGDLLIMPORT int ReservedBackends; +extern PGDLLIMPORT int SuperuserReservedConnections; extern PGDLLIMPORT int PostPortNumber; extern PGDLLIMPORT int Unix_socket_permissions; extern PGDLLIMPORT char *Unix_socket_group; -- 2.25.1 [text/x-diff] v4-0003-Introduce-reserved_connections-and-pg_use_reserve.patch (13.1K, ../../20230119175421.GA3934756@nathanxps13/4-v4-0003-Introduce-reserved_connections-and-pg_use_reserve.patch) download | inline diff: From 3c137c30d287ea7388d77ee7f3a5757227638c51 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Tue, 17 Jan 2023 15:36:59 -0800 Subject: [PATCH v4 3/3] Introduce reserved_connections and pg_use_reserved_connections. This provides a way to reserve connection slots for non-superusers. superuser_reserved_connections remains as a final reserve in case reserved_connections has been exhausted. --- doc/src/sgml/config.sgml | 39 ++++++++++++++++++- doc/src/sgml/user-manag.sgml | 5 +++ src/backend/postmaster/postmaster.c | 29 +++++++++----- src/backend/storage/lmgr/proc.c | 16 +++++--- src/backend/utils/init/postinit.c | 31 +++++++++++---- src/backend/utils/misc/guc_tables.c | 11 ++++++ src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/catalog/pg_authid.dat | 5 +++ src/include/postmaster/postmaster.h | 1 + src/include/storage/proc.h | 2 +- 10 files changed, 115 insertions(+), 25 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index e019a1aac9..626faa4d08 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -725,17 +725,52 @@ include_dir 'conf.d' number of active concurrent connections is at least <varname>max_connections</varname> minus <varname>superuser_reserved_connections</varname>, new - connections will be accepted only for superusers. + connections will be accepted only for superusers. The connection slots + reserved by this parameter are intended as final reserve for emergency + use after the slots reserved by + <xref linkend="guc-reserved-connections"/> have been exhausted. </para> <para> The default value is three connections. The value must be less - than <varname>max_connections</varname>. + than <varname>max_connections</varname> minus + <varname>reserved_connections</varname>. This parameter can only be set at server start. </para> </listitem> </varlistentry> + <varlistentry id="guc-reserved-connections" xreflabel="reserved_connections"> + <term><varname>reserved_connections</varname> (<type>integer</type>) + <indexterm> + <primary><varname>reserved_connections</varname> configuration parameter</primary> + </indexterm> + </term> + <listitem> + <para> + Determines the number of connection <quote>slots</quote> that are + reserved for connections by roles with privileges of the + <link linkend="predefined-roles-table"><literal>pg_used_reserved_connections</literal></link> + role. Whenever the number of free connection slots is greater than + <xref linkend="guc-superuser-reserved-connections"/> but less than or + equal to the sum of <varname>superuser_reserved_connections</varname> + and <varname>reserved_connections</varname>, new connections will be + accepted only for superusers and roles with privileges of + <literal>pg_use_reserved_connections</literal>. If + <varname>superuser_reserved_connections</varname> or fewer connection + slots are available, new connections will be accepted only for + superusers. + </para> + + <para> + The default value is zero connections. The value must be less than + <varname>max_connections</varname> minus + <varname>superuser_reserved_connections</varname>. This parameter can + only be set at server start. + </para> + </listitem> + </varlistentry> + <varlistentry id="guc-unix-socket-directories" xreflabel="unix_socket_directories"> <term><varname>unix_socket_directories</varname> (<type>string</type>) <indexterm> diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml index 71a2d8f298..002c1e3aff 100644 --- a/doc/src/sgml/user-manag.sgml +++ b/doc/src/sgml/user-manag.sgml @@ -689,6 +689,11 @@ DROP ROLE doomed_role; and <link linkend="sql-lock"><command>LOCK TABLE</command></link> on all relations.</entry> </row> + <row> + <entry>pg_use_reserved_connections</entry> + <entry>Allow use of connection slots reserved via + <xref linkend="guc-reserved-connections"/>.</entry> + </row> </tbody> </tgroup> </table> diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 3f799c4ac8..aca1ef91b5 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -205,14 +205,24 @@ char *ListenAddresses; /* * SuperuserReservedConnections is the number of backends reserved for - * superuser use. This number is taken out of the pool size given by - * MaxConnections so number of backend slots available to non-superusers is - * (MaxConnections - SuperuserReservedConnections). Note what this really - * means is "if there are <= SuperuserReservedConnections connections - * available, only superusers can make new connections" --- pre-existing - * superuser connections don't count against the limit. + * superuser use, and ReservedConnections is the number of backends reserved + * for use by roles with privileges of the pg_use_reserved_connections + * predefined role. These are taken out of the pool of MaxConnections backend + * slots, so the number of backend slots available for roles that are neither + * superuser nor have privileges of pg_use_reserved_connections is + * (MaxConnections - SuperuserReservedConnections - ReservedConnections). + * + * If the number of remaining slots is less than or equal to + * SuperuserReservedConnections, only superusers can make new connections. If + * the number of remaining slots is greater than SuperuserReservedConnections + * but less than or equal to + * (SuperuserReservedConnections + ReservedConnections), only superusers and + * roles with privileges of pg_use_reserved_connections can make new + * connections. Note that pre-existing superuser and + * pg_use_reserved_connections connections don't count against the limits. */ int SuperuserReservedConnections; +int ReservedConnections; /* The socket(s) we're listening to. */ #define MAXLISTEN 64 @@ -908,11 +918,12 @@ PostmasterMain(int argc, char *argv[]) /* * Check for invalid combinations of GUC settings. */ - if (SuperuserReservedConnections >= MaxConnections) + if (SuperuserReservedConnections + ReservedConnections >= MaxConnections) { - write_stderr("%s: superuser_reserved_connections (%d) must be less than max_connections (%d)\n", + write_stderr("%s: superuser_reserved_connections (%d) plus reserved_connections (%d) must be less than max_connections (%d)\n", progname, - SuperuserReservedConnections, MaxConnections); + SuperuserReservedConnections, ReservedConnections, + MaxConnections); ExitPostmaster(1); } if (XLogArchiveMode > ARCHIVE_MODE_OFF && wal_level == WAL_LEVEL_MINIMAL) diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index f8ac4edd6f..22b4278610 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -645,27 +645,33 @@ GetStartupBufferPinWaitBufId(void) } /* - * Check whether there are at least N free PGPROC objects. + * Check whether there are at least N free PGPROC objects. If false is + * returned, *nfree will be set to the number of free PGPROC objects. + * Otherwise, *nfree will be set to n. * * Note: this is designed on the assumption that N will generally be small. */ bool -HaveNFreeProcs(int n) +HaveNFreeProcs(int n, int *nfree) { dlist_iter iter; + Assert(n > 0); + Assert(nfree); + SpinLockAcquire(ProcStructLock); + *nfree = 0; dlist_foreach(iter, &ProcGlobal->freeProcs) { - n--; - if (n == 0) + (*nfree)++; + if (*nfree == n) break; } SpinLockRelease(ProcStructLock); - return (n <= 0); + return (*nfree == n); } /* diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 40f145e0ab..2f07ca7a0e 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -719,6 +719,7 @@ InitPostgres(const char *in_dbname, Oid dboid, bool am_superuser; char *fullpath; char dbname[NAMEDATALEN]; + int nfree = 0; elog(DEBUG3, "InitPostgres"); @@ -922,16 +923,30 @@ InitPostgres(const char *in_dbname, Oid dboid, } /* - * The last few connection slots are reserved for superusers. Replication - * connections are drawn from slots reserved with max_wal_senders and not - * limited by max_connections or superuser_reserved_connections. + * The last few connection slots are reserved for superusers and roles with + * privileges of pg_use_reserved_connections. Replication connections are + * drawn from slots reserved with max_wal_senders and are not limited by + * max_connections, superuser_reserved_connections, or + * reserved_connections. + * + * Note: At this point, the new backend has already claimed a proc struct, + * so we must check whether the number of free slots is strictly less than + * the reserved connection limits. */ if (!am_superuser && !am_walsender && - SuperuserReservedConnections > 0 && - !HaveNFreeProcs(SuperuserReservedConnections)) - ereport(FATAL, - (errcode(ERRCODE_TOO_MANY_CONNECTIONS), - errmsg("remaining connection slots are reserved for superusers"))); + (SuperuserReservedConnections + ReservedConnections) > 0 && + !HaveNFreeProcs(SuperuserReservedConnections + ReservedConnections, &nfree)) + { + if (nfree < SuperuserReservedConnections) + ereport(FATAL, + (errcode(ERRCODE_TOO_MANY_CONNECTIONS), + errmsg("remaining connection slots are reserved for superusers"))); + + if (!has_privs_of_role(GetUserId(), ROLE_PG_USE_RESERVED_CONNECTIONS)) + ereport(FATAL, + (errcode(ERRCODE_TOO_MANY_CONNECTIONS), + errmsg("remaining connection slots are reserved for roles with privileges of pg_use_reserved_connections"))); + } /* Check replication permissions needed for walsender processes. */ if (am_walsender) diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 0fa9fdd3c5..e1753a40fa 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -2168,6 +2168,17 @@ struct config_int ConfigureNamesInt[] = NULL, NULL, NULL }, + { + {"reserved_connections", PGC_POSTMASTER, CONN_AUTH_SETTINGS, + gettext_noop("Sets the number of connection slots reserved for roles " + "with privileges of pg_use_reserved_connections."), + NULL + }, + &ReservedConnections, + 0, 0, MAX_BACKENDS, + NULL, NULL, NULL + }, + { {"min_dynamic_shared_memory", PGC_POSTMASTER, RESOURCES_MEM, gettext_noop("Amount of dynamic shared memory reserved at startup."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 4cceda4162..f701312225 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -64,6 +64,7 @@ #port = 5432 # (change requires restart) #max_connections = 100 # (change requires restart) #superuser_reserved_connections = 3 # (change requires restart) +#reserved_connections = 0 # (change requires restart) #unix_socket_directories = '/tmp' # comma-separated list of directories # (change requires restart) #unix_socket_group = '' # (change requires restart) diff --git a/src/include/catalog/pg_authid.dat b/src/include/catalog/pg_authid.dat index 2a2fee7d28..f2e5663c9f 100644 --- a/src/include/catalog/pg_authid.dat +++ b/src/include/catalog/pg_authid.dat @@ -89,5 +89,10 @@ rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', rolpassword => '_null_', rolvaliduntil => '_null_' }, +{ oid => '4550', oid_symbol => 'ROLE_PG_USE_RESERVED_CONNECTIONS', + rolname => 'pg_use_reserved_connections', rolsuper => 'f', rolinherit => 't', + rolcreaterole => 'f', rolcreatedb => 'f', rolcanlogin => 'f', + rolreplication => 'f', rolbypassrls => 'f', rolconnlimit => '-1', + rolpassword => '_null_', rolvaliduntil => '_null_' }, ] diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h index 0e4b8ded34..3b3889c58c 100644 --- a/src/include/postmaster/postmaster.h +++ b/src/include/postmaster/postmaster.h @@ -16,6 +16,7 @@ /* GUC options */ extern PGDLLIMPORT bool EnableSSL; extern PGDLLIMPORT int SuperuserReservedConnections; +extern PGDLLIMPORT int ReservedConnections; extern PGDLLIMPORT int PostPortNumber; extern PGDLLIMPORT int Unix_socket_permissions; extern PGDLLIMPORT char *Unix_socket_group; diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index dd45b8ee9b..4258cd92c9 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -445,7 +445,7 @@ extern void InitAuxiliaryProcess(void); extern void SetStartupBufferPinWaitBufId(int bufid); extern int GetStartupBufferPinWaitBufId(void); -extern bool HaveNFreeProcs(int n); +extern bool HaveNFreeProcs(int n, int *nfree); extern void ProcReleaseLocks(bool isCommit); extern ProcWaitStatus ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable); -- 2.25.1 ^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet @ 2023-01-19 19:17 Robert Haas <[email protected]> parent: Nathan Bossart <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Robert Haas @ 2023-01-19 19:17 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: pgsql-hackers On Thu, Jan 19, 2023 at 12:54 PM Nathan Bossart <[email protected]> wrote: > > OK. Might be worth a short comment. > > I added one. Thanks. I'd move it to the inner indentation level so it's closer to the test at issue. I would also suggest reordering the documentation and the postgresql.conf.sample file so that reserved_connections precedes superuser_reserved_connections, instead of following it. Other than that, this seems like it might be about ready to commit, barring objections or bug reports. -- Robert Haas EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet @ 2023-01-20 13:34 tushar <[email protected]> parent: tushar <[email protected]> 1 sibling, 1 reply; 44+ messages in thread From: tushar @ 2023-01-20 13:34 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; Robert Haas <[email protected]>; +Cc: pgsql-hackers On 1/19/23 6:28 PM, tushar wrote: >> > There is one typo , for the doc changes, it is mentioned > "pg_use_reserved_backends" but i think it supposed to be > "pg_use_reserved_connections" > under Table 22.1. Predefined Roles. Thanks, this is fixed now with the latest patches. -- regards,tushar EnterpriseDB https://www.enterprisedb.com/ The Enterprise PostgreSQL Company ^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet @ 2023-01-20 18:10 Nathan Bossart <[email protected]> parent: tushar <[email protected]> 0 siblings, 1 reply; 44+ messages in thread From: Nathan Bossart @ 2023-01-20 18:10 UTC (permalink / raw) To: tushar <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers On Fri, Jan 20, 2023 at 07:04:58PM +0530, tushar wrote: > On 1/19/23 6:28 PM, tushar wrote: >> There is one typo , for the doc changes, it is mentioned >> "pg_use_reserved_backends" but i think it supposed to be >> "pg_use_reserved_connections" >> under Table 22.1. Predefined Roles. > > Thanks, this is fixed now with the latest patches. Thank you for reviewing. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet @ 2023-01-20 20:42 Robert Haas <[email protected]> parent: Nathan Bossart <[email protected]> 0 siblings, 1 reply; 44+ messages in thread From: Robert Haas @ 2023-01-20 20:42 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: tushar <[email protected]>; pgsql-hackers On Fri, Jan 20, 2023 at 1:10 PM Nathan Bossart <[email protected]> wrote: > > Thanks, this is fixed now with the latest patches. > > Thank you for reviewing. Thanks to you both. I have committed these patches. -- Robert Haas EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet @ 2023-01-20 21:02 Nathan Bossart <[email protected]> parent: Robert Haas <[email protected]> 0 siblings, 1 reply; 44+ messages in thread From: Nathan Bossart @ 2023-01-20 21:02 UTC (permalink / raw) To: Robert Haas <[email protected]>; +Cc: tushar <[email protected]>; pgsql-hackers On Fri, Jan 20, 2023 at 03:42:03PM -0500, Robert Haas wrote: > Thanks to you both. I have committed these patches. Thanks! Does this need a catversion bump? -- Nathan Bossart Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 44+ messages in thread
* Re: almost-super-user problems that we haven't fixed yet @ 2023-01-20 21:37 Robert Haas <[email protected]> parent: Nathan Bossart <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Robert Haas @ 2023-01-20 21:37 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: tushar <[email protected]>; pgsql-hackers On Fri, Jan 20, 2023 at 4:02 PM Nathan Bossart <[email protected]> wrote: > On Fri, Jan 20, 2023 at 03:42:03PM -0500, Robert Haas wrote: > > Thanks to you both. I have committed these patches. > > Thanks! Does this need a catversion bump? I was surprised by this question because I thought I'd included one. But it turns out I didn't include that in the commit and it's still in my working tree. *facepalm* -- Robert Haas EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH v17 2/3] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-06-30 17:41 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-06-30 17:41 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 51 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 8532bfd27e5..6eaa40f6acf 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,6 +482,31 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the xip array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. This difference does has no impact on XidInMVCCSnapshot(). + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + /* allocate in transaction context */ newxip = (TransactionId *) palloc(sizeof(TransactionId) * GetMaxSnapshotXidCount()); @@ -495,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -503,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -520,11 +542,22 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; - return snap; + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + } + result->snapshot_type = SNAPSHOT_MVCC; + + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index ea35f30f494..70a6b8902d1 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -212,7 +212,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -591,7 +590,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index d346be71642..147b190210a 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -60,6 +60,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.39.5 --ghyrlegy6t3fabqb Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v17-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH v17 2/3] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-06-30 17:41 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-06-30 17:41 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 51 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 8532bfd27e5..6eaa40f6acf 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,6 +482,31 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the xip array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. This difference does has no impact on XidInMVCCSnapshot(). + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + /* allocate in transaction context */ newxip = (TransactionId *) palloc(sizeof(TransactionId) * GetMaxSnapshotXidCount()); @@ -495,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -503,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -520,11 +542,22 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; - return snap; + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + } + result->snapshot_type = SNAPSHOT_MVCC; + + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index ea35f30f494..70a6b8902d1 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -212,7 +212,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -591,7 +590,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index d346be71642..147b190210a 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -60,6 +60,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.39.5 --ghyrlegy6t3fabqb Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v17-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH v19 4/6] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-08-11 13:23 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-08-11 13:23 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 51 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 98ddee20929..a2f1803622c 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,6 +482,31 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the xip array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. This difference does has no impact on XidInMVCCSnapshot(). + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + /* allocate in transaction context */ newxip = (TransactionId *) palloc(sizeof(TransactionId) * GetMaxSnapshotXidCount()); @@ -495,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -503,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -520,11 +542,22 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; - return snap; + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + } + result->snapshot_type = SNAPSHOT_MVCC; + + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 65561cc6bc3..bc7840052fe 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -212,7 +212,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -602,7 +601,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 604c1f90216..f65f83c85cd 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.39.5 --ipmcfwv2cm3fpxaf Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v19-0005-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/4] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-08-11 13:23 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-08-11 13:23 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 51 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 8532bfd27e5..6eaa40f6acf 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,6 +482,31 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the xip array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. This difference does has no impact on XidInMVCCSnapshot(). + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + /* allocate in transaction context */ newxip = (TransactionId *) palloc(sizeof(TransactionId) * GetMaxSnapshotXidCount()); @@ -495,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -503,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -520,11 +542,22 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index ea35f30f494..70a6b8902d1 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -212,7 +212,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -591,7 +590,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index d346be71642..147b190210a 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -60,6 +60,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.1 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v18-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH v19 4/6] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-08-11 13:23 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-08-11 13:23 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 51 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 98ddee20929..a2f1803622c 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,6 +482,31 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the xip array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. This difference does has no impact on XidInMVCCSnapshot(). + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + /* allocate in transaction context */ newxip = (TransactionId *) palloc(sizeof(TransactionId) * GetMaxSnapshotXidCount()); @@ -495,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -503,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -520,11 +542,22 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; - return snap; + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + } + result->snapshot_type = SNAPSHOT_MVCC; + + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 65561cc6bc3..bc7840052fe 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -212,7 +212,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -602,7 +601,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 604c1f90216..f65f83c85cd 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.39.5 --ipmcfwv2cm3fpxaf Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v19-0005-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH v20 4/6] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-08-11 13:23 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-08-11 13:23 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 51 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 98ddee20929..a2f1803622c 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,6 +482,31 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the xip array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. This difference does has no impact on XidInMVCCSnapshot(). + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + /* allocate in transaction context */ newxip = (TransactionId *) palloc(sizeof(TransactionId) * GetMaxSnapshotXidCount()); @@ -495,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -503,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -520,11 +542,22 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; - return snap; + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + } + result->snapshot_type = SNAPSHOT_MVCC; + + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 65561cc6bc3..bc7840052fe 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -212,7 +212,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -602,7 +601,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 604c1f90216..f65f83c85cd 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.39.5 --3q6txozlgl6t37td Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v20-0005-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH v25 3/4] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-08-11 13:23 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-08-11 13:23 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 51 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 98ddee20929..a2f1803622c 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,6 +482,31 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the xip array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. This difference does has no impact on XidInMVCCSnapshot(). + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + /* allocate in transaction context */ newxip = (TransactionId *) palloc(sizeof(TransactionId) * GetMaxSnapshotXidCount()); @@ -495,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -503,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -520,11 +542,22 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; - return snap; + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + } + result->snapshot_type = SNAPSHOT_MVCC; + + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 65561cc6bc3..bc7840052fe 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -212,7 +212,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -602,7 +601,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 604c1f90216..f65f83c85cd 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --ecl2euvoqutt27wj Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v25-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH v20 4/6] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-08-11 13:23 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-08-11 13:23 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 51 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 98ddee20929..a2f1803622c 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,6 +482,31 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the xip array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. This difference does has no impact on XidInMVCCSnapshot(). + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + /* allocate in transaction context */ newxip = (TransactionId *) palloc(sizeof(TransactionId) * GetMaxSnapshotXidCount()); @@ -495,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -503,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -520,11 +542,22 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; - return snap; + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + } + result->snapshot_type = SNAPSHOT_MVCC; + + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 65561cc6bc3..bc7840052fe 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -212,7 +212,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -602,7 +601,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 604c1f90216..f65f83c85cd 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.39.5 --3q6txozlgl6t37td Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v20-0005-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/4] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-08-11 13:23 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-08-11 13:23 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 51 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 8532bfd27e5..6eaa40f6acf 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,6 +482,31 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the xip array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. This difference does has no impact on XidInMVCCSnapshot(). + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + /* allocate in transaction context */ newxip = (TransactionId *) palloc(sizeof(TransactionId) * GetMaxSnapshotXidCount()); @@ -495,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -503,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -520,11 +542,22 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index ea35f30f494..70a6b8902d1 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -212,7 +212,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -591,7 +590,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index d346be71642..147b190210a 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -60,6 +60,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.1 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v18-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH v25 3/4] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-08-11 13:23 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-08-11 13:23 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 51 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 98ddee20929..a2f1803622c 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,6 +482,31 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the xip array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. This difference does has no impact on XidInMVCCSnapshot(). + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + /* allocate in transaction context */ newxip = (TransactionId *) palloc(sizeof(TransactionId) * GetMaxSnapshotXidCount()); @@ -495,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -503,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -520,11 +542,22 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; - return snap; + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + } + result->snapshot_type = SNAPSHOT_MVCC; + + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 65561cc6bc3..bc7840052fe 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -212,7 +212,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -602,7 +601,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 604c1f90216..f65f83c85cd 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --ecl2euvoqutt27wj Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v25-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/4] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-12-04 17:20 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-12-04 17:20 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 51 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 6e18baa33cb..dcf32101c4c 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,6 +482,31 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the xip array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. This difference does has no impact on XidInMVCCSnapshot(). + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + /* allocate in transaction context */ newxip = (TransactionId *) palloc(sizeof(TransactionId) * GetMaxSnapshotXidCount()); @@ -495,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -503,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -520,11 +542,22 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 24f73a49d27..886060305f5 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 604c1f90216..f65f83c85cd 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v26-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/4] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-12-04 17:20 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-12-04 17:20 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 51 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 6e18baa33cb..dcf32101c4c 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,6 +482,31 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the xip array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. This difference does has no impact on XidInMVCCSnapshot(). + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + /* allocate in transaction context */ newxip = (TransactionId *) palloc(sizeof(TransactionId) * GetMaxSnapshotXidCount()); @@ -495,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -503,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -520,11 +542,22 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 24f73a49d27..886060305f5 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 604c1f90216..f65f83c85cd 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v26-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-12-09 18:44 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-12-09 18:44 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/commands/cluster.c | 8 ++- src/backend/replication/logical/snapbuild.c | 57 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 5 files changed, 53 insertions(+), 17 deletions(-) diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 3afab656cd9..89f0b03a31c 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -70,8 +70,7 @@ typedef struct static bool cluster_rel_recheck(RepackCommand cmd, Relation OldHeap, Oid indexOid, Oid userid, int options); -static void rebuild_relation(RepackCommand cmd, - Relation OldHeap, Relation index, bool verbose); +static void rebuild_relation(Relation OldHeap, Relation index, bool verbose); static void copy_table_data(Relation NewHeap, Relation OldHeap, Relation OldIndex, bool verbose, bool *pSwapToastByContent, TransactionId *pFreezeXid, MultiXactId *pCutoffMulti); @@ -415,7 +414,7 @@ cluster_rel(RepackCommand cmd, Relation OldHeap, Oid indexOid, TransferPredicateLocksToHeapRelation(OldHeap); /* rebuild_relation does all the dirty work */ - rebuild_relation(cmd, OldHeap, index, verbose); + rebuild_relation(OldHeap, index, verbose); /* rebuild_relation closes OldHeap, and index if valid */ out: @@ -629,8 +628,7 @@ mark_index_clustered(Relation rel, Oid indexOid, bool is_internal) * On exit, they are closed, but locks on them are not released. */ static void -rebuild_relation(RepackCommand cmd, - Relation OldHeap, Relation index, bool verbose) +rebuild_relation(Relation OldHeap, Relation index, bool verbose) { Oid tableOid = RelationGetRelid(OldHeap); Oid accessMethod = OldHeap->rd_rel->relam; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 6e18baa33cb..34bdd987478 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,33 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + newxip = (TransactionId *) palloc(sizeof(TransactionId) * GetMaxSnapshotXidCount()); @@ -495,7 +518,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -503,7 +526,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -520,11 +543,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 24f73a49d27..886060305f5 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 604c1f90216..f65f83c85cd 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v27-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-12-09 18:44 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-12-09 18:44 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/commands/cluster.c | 8 ++- src/backend/replication/logical/snapbuild.c | 57 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 5 files changed, 53 insertions(+), 17 deletions(-) diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 3afab656cd9..89f0b03a31c 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -70,8 +70,7 @@ typedef struct static bool cluster_rel_recheck(RepackCommand cmd, Relation OldHeap, Oid indexOid, Oid userid, int options); -static void rebuild_relation(RepackCommand cmd, - Relation OldHeap, Relation index, bool verbose); +static void rebuild_relation(Relation OldHeap, Relation index, bool verbose); static void copy_table_data(Relation NewHeap, Relation OldHeap, Relation OldIndex, bool verbose, bool *pSwapToastByContent, TransactionId *pFreezeXid, MultiXactId *pCutoffMulti); @@ -415,7 +414,7 @@ cluster_rel(RepackCommand cmd, Relation OldHeap, Oid indexOid, TransferPredicateLocksToHeapRelation(OldHeap); /* rebuild_relation does all the dirty work */ - rebuild_relation(cmd, OldHeap, index, verbose); + rebuild_relation(OldHeap, index, verbose); /* rebuild_relation closes OldHeap, and index if valid */ out: @@ -629,8 +628,7 @@ mark_index_clustered(Relation rel, Oid indexOid, bool is_internal) * On exit, they are closed, but locks on them are not released. */ static void -rebuild_relation(RepackCommand cmd, - Relation OldHeap, Relation index, bool verbose) +rebuild_relation(Relation OldHeap, Relation index, bool verbose) { Oid tableOid = RelationGetRelid(OldHeap); Oid accessMethod = OldHeap->rd_rel->relam; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 6e18baa33cb..34bdd987478 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,33 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + newxip = (TransactionId *) palloc(sizeof(TransactionId) * GetMaxSnapshotXidCount()); @@ -495,7 +518,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -503,7 +526,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -520,11 +543,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 24f73a49d27..886060305f5 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 604c1f90216..f65f83c85cd 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v27-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-12-13 18:27 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-12-13 18:27 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 57 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index d6ab1e017eb..a3730804428 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,33 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +542,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 40a2e90e071..4cf32ffe833 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 604c1f90216..f65f83c85cd 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v28-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2025-12-13 18:27 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2025-12-13 18:27 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 57 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index d6ab1e017eb..a3730804428 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,33 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +542,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 40a2e90e071..4cf32ffe833 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 44031dcf6e3..6d4d2d1814c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 604c1f90216..f65f83c85cd 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v28-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-01-08 16:47 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-01-08 16:47 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 57 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..95f230f8e9b 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,33 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +542,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v29-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-01-08 16:47 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-01-08 16:47 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 57 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..95f230f8e9b 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,33 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +517,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +525,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +542,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v29-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-01-12 16:30 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-01-12 16:30 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 59 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..a738ad8a864 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,35 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + + Assert(snapshot->snapshot_type == SNAPSHOT_HISTORIC_MVCC); + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +519,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +527,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +544,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v30-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-01-12 16:30 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-01-12 16:30 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 59 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..a738ad8a864 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,35 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + + Assert(snapshot->snapshot_type == SNAPSHOT_HISTORIC_MVCC); + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +519,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +527,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +544,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v30-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-01-22 08:23 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-01-22 08:23 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 59 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..a738ad8a864 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,35 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + + Assert(snapshot->snapshot_type == SNAPSHOT_HISTORIC_MVCC); + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +519,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +527,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +544,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v31-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-01-22 08:23 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-01-22 08:23 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 59 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..a738ad8a864 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,35 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + + Assert(snapshot->snapshot_type == SNAPSHOT_HISTORIC_MVCC); + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +519,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +527,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +544,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v31-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH v33 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-01-27 10:48 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-01-27 10:48 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 59 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..a738ad8a864 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,35 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + + Assert(snapshot->snapshot_type == SNAPSHOT_HISTORIC_MVCC); + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +519,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +527,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +544,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; - return snap; + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; + + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --r7amdyq3rmktb267 Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v33-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-01-27 10:48 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-01-27 10:48 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 59 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..a738ad8a864 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,35 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + + Assert(snapshot->snapshot_type == SNAPSHOT_HISTORIC_MVCC); + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +519,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +527,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +544,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v32-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH v33 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-01-27 10:48 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-01-27 10:48 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 59 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..a738ad8a864 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,35 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + + Assert(snapshot->snapshot_type == SNAPSHOT_HISTORIC_MVCC); + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +519,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +527,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +544,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; - return snap; + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; + + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --r7amdyq3rmktb267 Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v33-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-01-27 10:48 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-01-27 10:48 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 59 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..a738ad8a864 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,35 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + + Assert(snapshot->snapshot_type == SNAPSHOT_HISTORIC_MVCC); + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +519,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +527,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +544,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v32-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH v35 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-02-16 19:49 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-02-16 19:49 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 59 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..a738ad8a864 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,35 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + + Assert(snapshot->snapshot_type == SNAPSHOT_HISTORIC_MVCC); + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +519,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +527,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +544,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; - return snap; + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; + + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --kmtyk4ln3767qrma Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v35-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-02-16 19:49 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-02-16 19:49 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 59 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..a738ad8a864 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,35 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + + Assert(snapshot->snapshot_type == SNAPSHOT_HISTORIC_MVCC); + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +519,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +527,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +544,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v34-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-02-16 19:49 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-02-16 19:49 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 59 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..a738ad8a864 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,35 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + + Assert(snapshot->snapshot_type == SNAPSHOT_HISTORIC_MVCC); + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +519,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +527,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +544,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v34-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH v35 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-02-16 19:49 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-02-16 19:49 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 59 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..a738ad8a864 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,35 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + + Assert(snapshot->snapshot_type == SNAPSHOT_HISTORIC_MVCC); + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +519,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +527,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +544,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; - return snap; + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; + + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --kmtyk4ln3767qrma Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v35-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-02-27 18:01 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-02-27 18:01 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 59 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..a738ad8a864 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,35 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + + Assert(snapshot->snapshot_type == SNAPSHOT_HISTORIC_MVCC); + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +519,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +527,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +544,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v36-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
* [PATCH 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. @ 2026-02-27 18:01 Antonin Houska <[email protected]> 0 siblings, 0 replies; 44+ messages in thread From: Antonin Houska @ 2026-02-27 18:01 UTC (permalink / raw) The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK CONCURRENTLY will also need it. --- src/backend/replication/logical/snapbuild.c | 59 +++++++++++++++++---- src/backend/utils/time/snapmgr.c | 3 +- src/include/replication/snapbuild.h | 1 + src/include/utils/snapmgr.h | 1 + 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 7f79621b57e..a738ad8a864 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -440,10 +440,7 @@ Snapshot SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; - TransactionId xid; TransactionId safeXid; - TransactionId *newxip; - int newxcnt = 0; Assert(XactIsoLevel == XACT_REPEATABLE_READ); Assert(builder->building_full_snapshot); @@ -485,7 +482,35 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ + /* Convert the historic snapshot to MVCC snapshot. */ + return SnapBuildMVCCFromHistoric(snap, true); +} + +/* + * Turn a historic MVCC snapshot into an ordinary MVCC snapshot. + * + * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this + * snapshot contains not only running main transactions, but also their + * subtransactions. On the other hand, 'subxip' will usually be empty. This + * difference does not affect the result of XidInMVCCSnapshot() because it + * searches both in 'xip' and 'subxip'. + * + * Pass true for 'in_place' if you don't care about modifying the source + * snapshot. If you need a new instance, and one that was allocated as a + * single chunk of memory, pass false. + */ +Snapshot +SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place) +{ + TransactionId xid; + TransactionId *oldxip = snapshot->xip; + uint32 oldxcnt = snapshot->xcnt; + TransactionId *newxip; + int newxcnt = 0; + Snapshot result; + + Assert(snapshot->snapshot_type == SNAPSHOT_HISTORIC_MVCC); + newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount()); /* @@ -494,7 +519,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ - for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) + for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);) { void *test; @@ -502,7 +527,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * Check whether transaction committed using the decoding snapshot * meaning of ->xip. */ - test = bsearch(&xid, snap->xip, snap->xcnt, + test = bsearch(&xid, snapshot->xip, snapshot->xcnt, sizeof(TransactionId), xidComparator); if (test == NULL) @@ -519,11 +544,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder) } /* adjust remaining snapshot fields as needed */ - snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snapshot->xcnt = newxcnt; + snapshot->xip = newxip; + + if (in_place) + result = snapshot; + else + { + result = CopySnapshot(snapshot); + + /* Restore the original values so the source is intact. */ + snapshot->xip = oldxip; + snapshot->xcnt = oldxcnt; + + /* newxip has been copied */ + pfree(newxip); + } + result->snapshot_type = SNAPSHOT_MVCC; - return snap; + return result; } /* diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 2e6197f5f35..3af1b366adf 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -213,7 +213,6 @@ typedef struct ExportedSnapshot static List *exportedSnapshots = NIL; /* Prototypes for local functions */ -static Snapshot CopySnapshot(Snapshot snapshot); static void UnregisterSnapshotNoOwner(Snapshot snapshot); static void FreeSnapshot(Snapshot snapshot); static void SnapshotResetXmin(void); @@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid, * The copy is palloc'd in TopTransactionContext and has initial refcounts set * to 0. The returned snapshot has the copied flag set. */ -static Snapshot +Snapshot CopySnapshot(Snapshot snapshot) { Snapshot newsnap; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index ccded021433..34383dea776 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder); extern void SnapBuildSnapDecRefcount(Snapshot snap); extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder); +extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place); extern const char *SnapBuildExportSnapshot(SnapBuild *builder); extern void SnapBuildClearExportedSnapshot(void); extern void SnapBuildResetExportedSnapshotState(void); diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index b8c01a291a1..de824945f0b 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void); extern Snapshot GetLatestSnapshot(void); extern void SnapshotSetCommandId(CommandId curcid); +extern Snapshot CopySnapshot(Snapshot snapshot); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); -- 2.47.3 --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=v36-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch ^ permalink raw reply [nested|flat] 44+ messages in thread
end of thread, other threads:[~2026-02-27 18:01 UTC | newest] Thread overview: 44+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2018-11-19 15:23 [PATCH] psql: Show IP address in \conninfo, if different from host Alvaro Herrera <[email protected]> 2023-01-18 21:14 Re: almost-super-user problems that we haven't fixed yet Nathan Bossart <[email protected]> 2023-01-19 12:58 ` Re: almost-super-user problems that we haven't fixed yet tushar <[email protected]> 2023-01-19 13:20 ` Re: almost-super-user problems that we haven't fixed yet tushar <[email protected]> 2023-01-20 13:34 ` Re: almost-super-user problems that we haven't fixed yet tushar <[email protected]> 2023-01-20 18:10 ` Re: almost-super-user problems that we haven't fixed yet Nathan Bossart <[email protected]> 2023-01-20 20:42 ` Re: almost-super-user problems that we haven't fixed yet Robert Haas <[email protected]> 2023-01-20 21:02 ` Re: almost-super-user problems that we haven't fixed yet Nathan Bossart <[email protected]> 2023-01-20 21:37 ` Re: almost-super-user problems that we haven't fixed yet Robert Haas <[email protected]> 2023-01-19 16:40 ` Re: almost-super-user problems that we haven't fixed yet Robert Haas <[email protected]> 2023-01-19 17:54 ` Re: almost-super-user problems that we haven't fixed yet Nathan Bossart <[email protected]> 2023-01-19 19:17 ` Re: almost-super-user problems that we haven't fixed yet Robert Haas <[email protected]> 2025-06-30 17:41 [PATCH v17 2/3] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2025-06-30 17:41 [PATCH v17 2/3] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2025-08-11 13:23 [PATCH v19 4/6] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2025-08-11 13:23 [PATCH v25 3/4] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2025-08-11 13:23 [PATCH 3/4] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2025-08-11 13:23 [PATCH v20 4/6] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2025-08-11 13:23 [PATCH v19 4/6] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2025-08-11 13:23 [PATCH v25 3/4] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2025-08-11 13:23 [PATCH 3/4] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2025-08-11 13:23 [PATCH v20 4/6] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2025-12-04 17:20 [PATCH 3/4] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2025-12-04 17:20 [PATCH 3/4] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2025-12-09 18:44 [PATCH 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2025-12-09 18:44 [PATCH 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2025-12-13 18:27 [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2025-12-13 18:27 [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-01-08 16:47 [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-01-08 16:47 [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-01-12 16:30 [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-01-12 16:30 [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-01-22 08:23 [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-01-22 08:23 [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-01-27 10:48 [PATCH v33 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-01-27 10:48 [PATCH v33 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-01-27 10:48 [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-01-27 10:48 [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-02-16 19:49 [PATCH 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-02-16 19:49 [PATCH v35 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-02-16 19:49 [PATCH v35 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-02-16 19:49 [PATCH 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-02-27 18:01 [PATCH 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]> 2026-02-27 18:01 [PATCH 3/5] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[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