On Tue, Aug 9, 2022 at 3:39 AM Drouvot, Bertrand <bdrou...@amazon.com> wrote:
> Agree that it makes sense to work on those patches in this particular
> order then.
Sounds good. The ClientConnectionInfo patch (previously 0002) is
attached, with the SQL function removed.
Thanks,
--Jacob
From a22ff3ba36f5eb93c582a957c7c2caca07ed21c5 Mon Sep 17 00:00:00 2001
From: Jacob Champion <pchamp...@vmware.com>
Date: Wed, 23 Mar 2022 15:07:05 -0700
Subject: [PATCH] Allow parallel workers to read authn_id
Move authn_id into a new global, MyClientConnectionInfo, which is
intended to hold all the client information that needs to be shared
between the backend and any parallel workers. MyClientConnectionInfo is
serialized and restored using a new parallel key.
---
src/backend/access/transam/parallel.c | 19 ++++++-
src/backend/libpq/auth.c | 16 +++---
src/backend/utils/init/miscinit.c | 72 +++++++++++++++++++++++++++
src/include/libpq/libpq-be.h | 39 ++++++++++-----
src/include/miscadmin.h | 4 ++
5 files changed, 129 insertions(+), 21 deletions(-)
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index df0cd77558..bc93101ff7 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -76,6 +76,7 @@
#define PARALLEL_KEY_REINDEX_STATE UINT64CONST(0xFFFFFFFFFFFF000C)
#define PARALLEL_KEY_RELMAPPER_STATE UINT64CONST(0xFFFFFFFFFFFF000D)
#define PARALLEL_KEY_UNCOMMITTEDENUMS UINT64CONST(0xFFFFFFFFFFFF000E)
+#define PARALLEL_KEY_CLIENTCONNINFO UINT64CONST(0xFFFFFFFFFFFF000F)
/* Fixed-size parallel state. */
typedef struct FixedParallelState
@@ -212,6 +213,7 @@ InitializeParallelDSM(ParallelContext *pcxt)
Size reindexlen = 0;
Size relmapperlen = 0;
Size uncommittedenumslen = 0;
+ Size clientconninfolen = 0;
Size segsize = 0;
int i;
FixedParallelState *fps;
@@ -272,8 +274,10 @@ InitializeParallelDSM(ParallelContext *pcxt)
shm_toc_estimate_chunk(&pcxt->estimator, relmapperlen);
uncommittedenumslen = EstimateUncommittedEnumsSpace();
shm_toc_estimate_chunk(&pcxt->estimator, uncommittedenumslen);
+ clientconninfolen = EstimateClientConnectionInfoSpace();
+ shm_toc_estimate_chunk(&pcxt->estimator, clientconninfolen);
/* If you add more chunks here, you probably need to add keys. */
- shm_toc_estimate_keys(&pcxt->estimator, 11);
+ shm_toc_estimate_keys(&pcxt->estimator, 12);
/* Estimate space need for error queues. */
StaticAssertStmt(BUFFERALIGN(PARALLEL_ERROR_QUEUE_SIZE) ==
@@ -352,6 +356,7 @@ InitializeParallelDSM(ParallelContext *pcxt)
char *session_dsm_handle_space;
char *entrypointstate;
char *uncommittedenumsspace;
+ char *clientconninfospace;
Size lnamelen;
/* Serialize shared libraries we have loaded. */
@@ -422,6 +427,12 @@ InitializeParallelDSM(ParallelContext *pcxt)
shm_toc_insert(pcxt->toc, PARALLEL_KEY_UNCOMMITTEDENUMS,
uncommittedenumsspace);
+ /* Serialize our ClientConnectionInfo. */
+ clientconninfospace = shm_toc_allocate(pcxt->toc, clientconninfolen);
+ SerializeClientConnectionInfo(clientconninfolen, clientconninfospace);
+ shm_toc_insert(pcxt->toc, PARALLEL_KEY_CLIENTCONNINFO,
+ clientconninfospace);
+
/* Allocate space for worker information. */
pcxt->worker = palloc0(sizeof(ParallelWorkerInfo) * pcxt->nworkers);
@@ -1270,6 +1281,7 @@ ParallelWorkerMain(Datum main_arg)
char *reindexspace;
char *relmapperspace;
char *uncommittedenumsspace;
+ char *clientconninfospace;
StringInfoData msgbuf;
char *session_dsm_handle_space;
Snapshot tsnapshot;
@@ -1479,6 +1491,11 @@ ParallelWorkerMain(Datum main_arg)
false);
RestoreUncommittedEnums(uncommittedenumsspace);
+ /* Restore the ClientConnectionInfo. */
+ clientconninfospace = shm_toc_lookup(toc, PARALLEL_KEY_CLIENTCONNINFO,
+ false);
+ RestoreClientConnectionInfo(clientconninfospace);
+
/* Attach to the leader's serializable transaction, if SERIALIZABLE. */
AttachSerializableXact(fps->serializable_xact_handle);
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 2d9ab7edce..313a6ea701 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -342,15 +342,15 @@ auth_failed(Port *port, int status, const char *logdetail)
* authorization will fail later.
*
* The provided string will be copied into TopMemoryContext, to match the
- * lifetime of the Port, so it is safe to pass a string that is managed by an
- * external library.
+ * lifetime of MyClientConnectionInfo, so it is safe to pass a string that is
+ * managed by an external library.
*/
static void
set_authn_id(Port *port, const char *id)
{
Assert(id);
- if (port->authn_id)
+ if (MyClientConnectionInfo.authn_id)
{
/*
* An existing authn_id should never be overwritten; that means two
@@ -361,17 +361,18 @@ set_authn_id(Port *port, const char *id)
ereport(FATAL,
(errmsg("authentication identifier set more than once"),
errdetail_log("previous identifier: \"%s\"; new identifier: \"%s\"",
- port->authn_id, id)));
+ MyClientConnectionInfo.authn_id, id)));
}
- port->authn_id = MemoryContextStrdup(TopMemoryContext, id);
+ MyClientConnectionInfo.authn_id = MemoryContextStrdup(TopMemoryContext, id);
if (Log_connections)
{
ereport(LOG,
errmsg("connection authenticated: identity=\"%s\" method=%s "
"(%s:%d)",
- port->authn_id, hba_authname(port->hba->auth_method), HbaFileName,
+ MyClientConnectionInfo.authn_id,
+ hba_authname(port->hba->auth_method), HbaFileName,
port->hba->linenumber));
}
}
@@ -1908,7 +1909,8 @@ auth_peer(hbaPort *port)
*/
set_authn_id(port, pw->pw_name);
- ret = check_usermap(port->hba->usermap, port->user_name, port->authn_id, false);
+ ret = check_usermap(port->hba->usermap, port->user_name,
+ MyClientConnectionInfo.authn_id, false);
return ret;
#else
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index eb43b2c5e5..973103374b 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -931,6 +931,78 @@ GetUserNameFromId(Oid roleid, bool noerr)
return result;
}
+/* ------------------------------------------------------------------------
+ * Parallel connection state
+ *
+ * ClientConnectionInfo contains pieces of information about the client that
+ * need to be synced to parallel workers when they initialize. Over time, this
+ * list will probably grow, and may subsume some of the "user state" variables
+ * above.
+ *-------------------------------------------------------------------------
+ */
+
+ClientConnectionInfo MyClientConnectionInfo;
+
+/*
+ * Calculate the space needed to serialize MyClientConnectionInfo.
+ */
+Size
+EstimateClientConnectionInfoSpace(void)
+{
+ Size size = 1;
+
+ if (MyClientConnectionInfo.authn_id)
+ size = add_size(size, strlen(MyClientConnectionInfo.authn_id) + 1);
+
+ return size;
+}
+
+/*
+ * Serialize MyClientConnectionInfo for use by parallel workers.
+ */
+void
+SerializeClientConnectionInfo(Size maxsize, char *start_address)
+{
+ /*
+ * First byte is an indication of whether or not authn_id has been set to
+ * non-NULL, to differentiate that case from the empty string.
+ */
+ Assert(maxsize > 0);
+ start_address[0] = MyClientConnectionInfo.authn_id ? 1 : 0;
+ start_address++;
+ maxsize--;
+
+ if (MyClientConnectionInfo.authn_id)
+ {
+ Size len;
+
+ len = strlcpy(start_address, MyClientConnectionInfo.authn_id, maxsize) + 1;
+ Assert(len <= maxsize);
+ maxsize -= len;
+ start_address += len;
+ }
+}
+
+/*
+ * Restore MyClientConnectionInfo from its serialized representation.
+ */
+void
+RestoreClientConnectionInfo(char *conninfo)
+{
+ if (conninfo[0] == 0)
+ {
+ MyClientConnectionInfo.authn_id = NULL;
+ conninfo++;
+ }
+ else
+ {
+ conninfo++;
+ MyClientConnectionInfo.authn_id = MemoryContextStrdup(TopMemoryContext,
+ conninfo);
+ conninfo += strlen(conninfo) + 1;
+ }
+}
+
/*-------------------------------------------------------------------------
* Interlock-file support
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 90c20da22b..c900411fdd 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -98,6 +98,31 @@ typedef struct
} pg_gssinfo;
#endif
+/*
+ * Fields describing the client connection, that also need to be copied over to
+ * parallel workers, go into the ClientConnectionInfo rather than Port. The same
+ * rules apply for allocations here as for Port (must be malloc'd or palloc'd in
+ * TopMemoryContext).
+ *
+ * If you add a struct member here, remember to also handle serialization in
+ * SerializeClientConnectionInfo() et al.
+ */
+typedef struct
+{
+ /*
+ * Authenticated identity. The meaning of this identifier is dependent on
+ * hba->auth_method; it is the identity (if any) that the user presented
+ * during the authentication cycle, before they were assigned a database
+ * role. (It is effectively the "SYSTEM-USERNAME" of a pg_ident usermap
+ * -- though the exact string in use may be different, depending on pg_hba
+ * options.)
+ *
+ * authn_id is NULL if the user has not actually been authenticated, for
+ * example if the "trust" auth method is in use.
+ */
+ const char *authn_id;
+} ClientConnectionInfo;
+
/*
* This is used by the postmaster in its communication with frontends. It
* contains all state information needed during this communication before the
@@ -158,19 +183,6 @@ typedef struct Port
*/
HbaLine *hba;
- /*
- * Authenticated identity. The meaning of this identifier is dependent on
- * hba->auth_method; it is the identity (if any) that the user presented
- * during the authentication cycle, before they were assigned a database
- * role. (It is effectively the "SYSTEM-USERNAME" of a pg_ident usermap
- * -- though the exact string in use may be different, depending on pg_hba
- * options.)
- *
- * authn_id is NULL if the user has not actually been authenticated, for
- * example if the "trust" auth method is in use.
- */
- const char *authn_id;
-
/*
* TCP keepalive and user timeout settings.
*
@@ -327,6 +339,7 @@ extern ssize_t be_gssapi_write(Port *port, void *ptr, size_t len);
#endif /* ENABLE_GSS */
extern PGDLLIMPORT ProtocolVersion FrontendProtocol;
+extern PGDLLIMPORT ClientConnectionInfo MyClientConnectionInfo;
/* TCP keepalives configuration. These are no-ops on an AF_UNIX socket. */
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 067b729d5a..3e9297e399 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -481,6 +481,10 @@ extern bool has_rolreplication(Oid roleid);
typedef void (*shmem_request_hook_type) (void);
extern PGDLLIMPORT shmem_request_hook_type shmem_request_hook;
+extern Size EstimateClientConnectionInfoSpace(void);
+extern void SerializeClientConnectionInfo(Size maxsize, char *start_address);
+extern void RestoreClientConnectionInfo(char *procinfo);
+
/* in executor/nodeHash.c */
extern size_t get_hash_memory_limit(void);
--
2.25.1
