diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c index 0a1e089ec1d..60d95037b36 100644 --- a/src/backend/access/transam/parallel.c +++ b/src/backend/access/transam/parallel.c @@ -1419,10 +1419,16 @@ ParallelWorkerMain(Datum main_arg) fps->session_user_is_superuser); SetCurrentRoleId(fps->outer_user_id, fps->role_is_superuser); - /* Restore database connection. */ + /* + * Restore database connection. We skip connection authorization checks, + * reasoning that (a) the leader checked these things when it started, and + * (b) we do not want parallel mode to cause these failures, because that + * would make use of parallel query plans not transparent to applications. + */ BackgroundWorkerInitializeConnectionByOid(fps->database_id, fps->authenticated_user_id, - 0); + BGWORKER_BYPASS_ALLOWCONN | + BGWORKER_BYPASS_ROLELOGINCHECK); /* * Set the client encoding to the database encoding, since that is what diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 49be1df91c1..edb8e060814 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -466,7 +466,7 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid, proc->databaseId = databaseid; proc->roleId = owner; proc->tempNamespaceId = InvalidOid; - proc->isBackgroundWorker = false; + proc->isBackgroundWorker = true; proc->lwWaiting = LW_WS_NOT_WAITING; proc->lwWaitMode = 0; proc->waitLock = NULL; diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 8078eeef62e..6d324af7b0e 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -1551,12 +1551,16 @@ AutoVacWorkerMain(char *startup_data, size_t startup_data_len) pgstat_report_autovac(dbid); /* - * Connect to the selected database, specifying no particular user + * Connect to the selected database, specifying no particular user, + * and ignoring datallowconn. Collect the database's name for + * display. * * Note: if we have selected a just-deleted database (due to using * stale stats info), we'll fail and exit here. */ - InitPostgres(NULL, dbid, NULL, InvalidOid, 0, dbname); + InitPostgres(NULL, dbid, NULL, InvalidOid, + INIT_PG_OVERRIDE_ALLOW_CONNS, + dbname); SetProcessingMode(NormalProcessing); set_ps_display(dbname); ereport(DEBUG1, diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c index 07bc5517fc2..7afe56885cb 100644 --- a/src/backend/postmaster/bgworker.c +++ b/src/backend/postmaster/bgworker.c @@ -854,7 +854,7 @@ BackgroundWorkerInitializeConnection(const char *dbname, const char *username, u BackgroundWorker *worker = MyBgworkerEntry; bits32 init_flags = 0; /* never honor session_preload_libraries */ - /* ignore datallowconn? */ + /* ignore datallowconn and ACL_CONNECT? */ if (flags & BGWORKER_BYPASS_ALLOWCONN) init_flags |= INIT_PG_OVERRIDE_ALLOW_CONNS; /* ignore rolcanlogin? */ @@ -888,7 +888,7 @@ BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, uint32 flags) BackgroundWorker *worker = MyBgworkerEntry; bits32 init_flags = 0; /* never honor session_preload_libraries */ - /* ignore datallowconn? */ + /* ignore datallowconn and ACL_CONNECT? */ if (flags & BGWORKER_BYPASS_ALLOWCONN) init_flags |= INIT_PG_OVERRIDE_ALLOW_CONNS; /* ignore rolcanlogin? */ diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index c769b1aa3ef..a640b6f5f78 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -3622,8 +3622,7 @@ CountDBBackends(Oid databaseid) } /* - * CountDBConnections --- counts database backends ignoring any background - * worker processes + * CountDBConnections --- counts database backends (only regular backends) */ int CountDBConnections(Oid databaseid) @@ -3695,6 +3694,7 @@ CancelDBBackends(Oid databaseid, ProcSignalReason sigmode, bool conflictPending) /* * CountUserBackends --- count backends that are used by specified user + * (only regular backends, not any type of background worker) */ int CountUserBackends(Oid roleid) diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 10d4fb4ea1a..acf16d2b89f 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -432,7 +432,7 @@ InitProcess(void) MyProc->databaseId = InvalidOid; MyProc->roleId = InvalidOid; MyProc->tempNamespaceId = InvalidOid; - MyProc->isBackgroundWorker = AmBackgroundWorkerProcess(); + MyProc->isBackgroundWorker = !AmRegularBackendProcess(); MyProc->delayChkptFlags = 0; MyProc->statusFlags = 0; /* NB -- autovac launcher intentionally does not set IS_AUTOVACUUM */ @@ -631,7 +631,7 @@ InitAuxiliaryProcess(void) MyProc->databaseId = InvalidOid; MyProc->roleId = InvalidOid; MyProc->tempNamespaceId = InvalidOid; - MyProc->isBackgroundWorker = AmBackgroundWorkerProcess(); + MyProc->isBackgroundWorker = true; MyProc->delayChkptFlags = 0; MyProc->statusFlags = 0; MyProc->lwWaiting = LW_WS_NOT_WAITING; diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index d24ac133fb7..6349abb8fb6 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -755,13 +755,27 @@ has_rolreplication(Oid roleid) * Initialize user identity during normal backend startup */ void -InitializeSessionUserId(const char *rolename, Oid roleid, bool bypass_login_check) +InitializeSessionUserId(const char *rolename, Oid roleid, + bool bypass_login_check) { HeapTuple roleTup; Form_pg_authid rform; char *rname; bool is_superuser; + /* + * In a parallel worker, we don't have to do anything here. + * ParallelWorkerMain already set our output variables, and we aren't + * going to enforce either rolcanlogin or rolconnlimit. Furthermore, we + * don't really want to perform a catalog lookup for the role: we don't + * want to fail if it's been dropped. + */ + if (InitializingParallelWorker) + { + Assert(bypass_login_check); + return; + } + /* * Don't do scans if we're bootstrapping, none of the system catalogs * exist yet, and they should be owned by postgres anyway. @@ -777,34 +791,22 @@ InitializeSessionUserId(const char *rolename, Oid roleid, bool bypass_login_chec /* * Look up the role, either by name if that's given or by OID if not. - * Normally we have to fail if we don't find it, but in parallel workers - * just return without doing anything: all the critical work has been done - * already. The upshot of that is that if the role has been deleted, we - * will not enforce its rolconnlimit against parallel workers anymore. */ if (rolename != NULL) { roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum(rolename)); if (!HeapTupleIsValid(roleTup)) - { - if (InitializingParallelWorker) - return; ereport(FATAL, (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION), errmsg("role \"%s\" does not exist", rolename))); - } } else { roleTup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid)); if (!HeapTupleIsValid(roleTup)) - { - if (InitializingParallelWorker) - return; ereport(FATAL, (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION), errmsg("role with OID %u does not exist", roleid))); - } } rform = (Form_pg_authid) GETSTRUCT(roleTup); @@ -812,33 +814,29 @@ InitializeSessionUserId(const char *rolename, Oid roleid, bool bypass_login_chec rname = NameStr(rform->rolname); is_superuser = rform->rolsuper; - /* In a parallel worker, ParallelWorkerMain already set these variables */ - if (!InitializingParallelWorker) - { - SetAuthenticatedUserId(roleid); + SetAuthenticatedUserId(roleid); - /* - * Set SessionUserId and related variables, including "role", via the - * GUC mechanisms. - * - * Note: ideally we would use PGC_S_DYNAMIC_DEFAULT here, so that - * session_authorization could subsequently be changed from - * pg_db_role_setting entries. Instead, session_authorization in - * pg_db_role_setting has no effect. Changing that would require - * solving two problems: - * - * 1. If pg_db_role_setting has values for both session_authorization - * and role, we could not be sure which order those would be applied - * in, and it would matter. - * - * 2. Sites may have years-old session_authorization entries. There's - * not been any particular reason to remove them. Ending the dormancy - * of those entries could seriously change application behavior, so - * only a major release should do that. - */ - SetConfigOption("session_authorization", rname, - PGC_BACKEND, PGC_S_OVERRIDE); - } + /* + * Set SessionUserId and related variables, including "role", via the GUC + * mechanisms. + * + * Note: ideally we would use PGC_S_DYNAMIC_DEFAULT here, so that + * session_authorization could subsequently be changed from + * pg_db_role_setting entries. Instead, session_authorization in + * pg_db_role_setting has no effect. Changing that would require solving + * two problems: + * + * 1. If pg_db_role_setting has values for both session_authorization and + * role, we could not be sure which order those would be applied in, and + * it would matter. + * + * 2. Sites may have years-old session_authorization entries. There's not + * been any particular reason to remove them. Ending the dormancy of + * those entries could seriously change application behavior, so only a + * major release should do that. + */ + SetConfigOption("session_authorization", rname, + PGC_BACKEND, PGC_S_OVERRIDE); /* * These next checks are not enforced when in standalone mode, so that @@ -848,7 +846,8 @@ InitializeSessionUserId(const char *rolename, Oid roleid, bool bypass_login_chec if (IsUnderPostmaster) { /* - * Is role allowed to login at all? + * Is role allowed to login at all? (But background workers can + * override this by setting bypass_login_check.) */ if (!bypass_login_check && !rform->rolcanlogin) ereport(FATAL, @@ -857,7 +856,9 @@ InitializeSessionUserId(const char *rolename, Oid roleid, bool bypass_login_chec rname))); /* - * Check connection limit for this role. + * Check connection limit for this role. We enforce the limit only + * for regular backends, since other process types have their own + * PGPROC pools. * * There is a race condition here --- we create our PGPROC before * checking for other PGPROCs. If two backends did this at about the @@ -867,6 +868,7 @@ InitializeSessionUserId(const char *rolename, Oid roleid, bool bypass_login_chec * just document that the connection limit is approximate. */ if (rform->rolconnlimit >= 0 && + AmRegularBackendProcess() && !is_superuser && CountUserBackends(roleid) > rform->rolconnlimit) ereport(FATAL, diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 9731de5781d..01c4016ced6 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -22,7 +22,6 @@ #include "access/genam.h" #include "access/heapam.h" #include "access/htup_details.h" -#include "access/parallel.h" #include "access/session.h" #include "access/tableam.h" #include "access/xact.h" @@ -341,13 +340,13 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect * These checks are not enforced when in standalone mode, so that there is * a way to recover from disabling all access to all databases, for * example "UPDATE pg_database SET datallowconn = false;". - * - * We do not enforce them for autovacuum worker processes either. */ - if (IsUnderPostmaster && !AmAutoVacuumWorkerProcess()) + if (IsUnderPostmaster) { /* * Check that the database is currently allowing connections. + * (Background processes can override this test and the next one by + * setting override_allow_connections.) */ if (!dbform->datallowconn && !override_allow_connections) ereport(FATAL, @@ -360,7 +359,7 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect * is redundant, but since we have the flag, might as well check it * and save a few cycles.) */ - if (!am_superuser && + if (!am_superuser && !override_allow_connections && object_aclcheck(DatabaseRelationId, MyDatabaseId, GetUserId(), ACL_CONNECT) != ACLCHECK_OK) ereport(FATAL, @@ -369,7 +368,9 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect errdetail("User does not have CONNECT privilege."))); /* - * Check connection limit for this database. + * Check connection limit for this database. We enforce the limit + * only for regular backends, since other process types have their own + * PGPROC pools. * * There is a race condition here --- we create our PGPROC before * checking for other PGPROCs. If two backends did this at about the @@ -379,6 +380,7 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect * just document that the connection limit is approximate. */ if (dbform->datconnlimit >= 0 && + AmRegularBackendProcess() && !am_superuser && CountDBConnections(MyDatabaseId) > dbform->datconnlimit) ereport(FATAL, @@ -865,23 +867,7 @@ InitPostgres(const char *in_dbname, Oid dboid, { InitializeSessionUserId(username, useroid, (flags & INIT_PG_OVERRIDE_ROLE_LOGIN) != 0); - - /* - * In a parallel worker, set am_superuser based on the - * authenticated user ID, not the current role. This is pretty - * dubious but it matches our historical behavior. Note that this - * value of am_superuser is used only for connection-privilege - * checks here and in CheckMyDatabase (we won't reach - * process_startup_options in a background worker). - * - * In other cases, there's been no opportunity for the current - * role to diverge from the authenticated user ID yet, so we can - * just rely on superuser() and avoid an extra catalog lookup. - */ - if (InitializingParallelWorker) - am_superuser = superuser_arg(GetAuthenticatedUserId()); - else - am_superuser = superuser(); + am_superuser = superuser(); } } else @@ -908,17 +894,16 @@ InitPostgres(const char *in_dbname, Oid dboid, } /* - * 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. + * The last few regular connection slots are reserved for superusers and + * roles with privileges of pg_use_reserved_connections. We do not apply + * these limits to background processes, since they all have their own + * pools of PGPROC slots. * * 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 && + if (AmRegularBackendProcess() && !am_superuser && (SuperuserReservedConnections + ReservedConnections) > 0 && !HaveNFreeProcs(SuperuserReservedConnections + ReservedConnections, &nfree)) { diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index d07181f9321..e4c0d1481e9 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -376,6 +376,7 @@ typedef enum BackendType extern PGDLLIMPORT BackendType MyBackendType; +#define AmRegularBackendProcess() (MyBackendType == B_BACKEND) #define AmAutoVacuumLauncherProcess() (MyBackendType == B_AUTOVAC_LAUNCHER) #define AmAutoVacuumWorkerProcess() (MyBackendType == B_AUTOVAC_WORKER) #define AmBackgroundWorkerProcess() (MyBackendType == B_BG_WORKER) diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 14e34d4e933..b20f9989a3f 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -216,7 +216,7 @@ struct PGPROC Oid tempNamespaceId; /* OID of temp schema this backend is * using */ - bool isBackgroundWorker; /* true if background worker. */ + bool isBackgroundWorker; /* true if not a regular backend. */ /* * While in hot standby mode, shows that a conflict signal has been sent diff --git a/src/test/modules/worker_spi/worker_spi.c b/src/test/modules/worker_spi/worker_spi.c index d4403b24d98..cf5b7505ec7 100644 --- a/src/test/modules/worker_spi/worker_spi.c +++ b/src/test/modules/worker_spi/worker_spi.c @@ -169,16 +169,6 @@ worker_spi_main(Datum main_arg) BackgroundWorkerInitializeConnection(worker_spi_database, worker_spi_role, flags); - /* - * Disable parallel query for workers started with - * BGWORKER_BYPASS_ALLOWCONN or BGWORKER_BYPASS_ROLELOGINCHECK so as these - * don't attempt connections using a database or a role that may not allow - * that. - */ - if ((flags & (BGWORKER_BYPASS_ALLOWCONN | BGWORKER_BYPASS_ROLELOGINCHECK))) - SetConfigOption("max_parallel_workers_per_gather", "0", - PGC_USERSET, PGC_S_OVERRIDE); - elog(LOG, "%s initialized with %s.%s", MyBgworkerEntry->bgw_name, table->schema, table->name); initialize_worker_spi(table);