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