On Thu, Feb 22, 2018 at 7:24 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Magnus Hagander <mag...@hagander.net> writes:
> > Attached is a patch that adds new Override versions of the functions to
> > connect to a database from a background worker.
>
> > Another option would be to just add the parameter directly to the regular
> > connection function, and not create separate functions. But that would
> make
> > it an incompatible change. And since background workers are commonly used
> > in extensions, that would break a lot of extensions out there. I figured
> > it's probably not worth doing that, and thus added the new functions.
> What
> > do others think about that?
>
> Meh. We change exported APIs in new major versions all the time. As
> long as it's just a question of an added parameter, people can deal
> with it. You could take the opportunity to future-proof a little by
> making this option be the first bit in a flags parameter, so that at
> least future boolean option additions don't require another API break
> or a whole new set of redundant functions.
>
Fair enough. In that case, something like the attached?
> Are there any other caveats in doing that this actually makes it dangerous
> > to just allow bypassing it for extensions?
>
> Don't think so; we autovacuum such DBs anyway don't we?
>
Yes. We set the freeze ages to 0 but we do run on it.
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c
index f99f9c07af..bb28e237d1 100644
--- a/contrib/pg_prewarm/autoprewarm.c
+++ b/contrib/pg_prewarm/autoprewarm.c
@@ -445,7 +445,7 @@ autoprewarm_database_main(Datum main_arg)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("could not map dynamic shared memory segment")));
- BackgroundWorkerInitializeConnectionByOid(apw_state->database, InvalidOid);
+ BackgroundWorkerInitializeConnectionByOid(apw_state->database, InvalidOid, 0);
block_info = (BlockInfoRecord *) dsm_segment_address(seg);
pos = apw_state->prewarm_start_idx;
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 9d4efc0f8f..1d631b7275 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -1324,7 +1324,8 @@ ParallelWorkerMain(Datum main_arg)
/* Restore database connection. */
BackgroundWorkerInitializeConnectionByOid(fps->database_id,
- fps->authenticated_user_id);
+ fps->authenticated_user_id,
+ 0);
/*
* Set the client encoding to the database encoding, since that is what
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 28ff2f0979..1430894ad2 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -498,7 +498,7 @@ BootstrapModeMain(void)
*/
InitProcess();
- InitPostgres(NULL, InvalidOid, NULL, InvalidOid, NULL);
+ InitPostgres(NULL, InvalidOid, NULL, InvalidOid, NULL, false);
/* Initialize stuff for bootstrap-file processing */
for (i = 0; i < MAXATTR; i++)
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 702f8d8188..a52178f7d3 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -477,7 +477,7 @@ AutoVacLauncherMain(int argc, char *argv[])
InitProcess();
#endif
- InitPostgres(NULL, InvalidOid, NULL, InvalidOid, NULL);
+ InitPostgres(NULL, InvalidOid, NULL, InvalidOid, NULL, false);
SetProcessingMode(NormalProcessing);
@@ -1679,7 +1679,7 @@ AutoVacWorkerMain(int argc, char *argv[])
* 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, dbname);
+ InitPostgres(NULL, dbid, NULL, InvalidOid, dbname, false);
SetProcessingMode(NormalProcessing);
set_ps_display(dbname, false);
ereport(DEBUG1,
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index f3ddf828bb..6f1b621f70 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -5585,7 +5585,7 @@ MaxLivePostmasterChildren(void)
* Connect background worker to a database.
*/
void
-BackgroundWorkerInitializeConnection(const char *dbname, const char *username)
+BackgroundWorkerInitializeConnection(const char *dbname, const char *username, uint32 flags)
{
BackgroundWorker *worker = MyBgworkerEntry;
@@ -5595,7 +5595,7 @@ BackgroundWorkerInitializeConnection(const char *dbname, const char *username)
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
errmsg("database connection requirement not indicated during registration")));
- InitPostgres(dbname, InvalidOid, username, InvalidOid, NULL);
+ InitPostgres(dbname, InvalidOid, username, InvalidOid, NULL, (flags & BGWORKER_BYPASS_ALLOWCONN) != 0);
/* it had better not gotten out of "init" mode yet */
if (!IsInitProcessingMode())
@@ -5608,7 +5608,7 @@ BackgroundWorkerInitializeConnection(const char *dbname, const char *username)
* Connect background worker to a database using OIDs.
*/
void
-BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid)
+BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, uint32 flags)
{
BackgroundWorker *worker = MyBgworkerEntry;
@@ -5618,7 +5618,7 @@ BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid)
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
errmsg("database connection requirement not indicated during registration")));
- InitPostgres(NULL, dboid, NULL, useroid, NULL);
+ InitPostgres(NULL, dboid, NULL, useroid, NULL, (flags & BGWORKER_BYPASS_ALLOWCONN) != 0);
/* it had better not gotten out of "init" mode yet */
if (!IsInitProcessingMode())
diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index 2da9129562..6ef333b725 100644
--- a/src/backend/replication/logical/launcher.c
+++ b/src/backend/replication/logical/launcher.c
@@ -901,7 +901,7 @@ ApplyLauncherMain(Datum main_arg)
* Establish connection to nailed catalogs (we only ever access
* pg_subscription).
*/
- BackgroundWorkerInitializeConnection(NULL, NULL);
+ BackgroundWorkerInitializeConnection(NULL, NULL, 0);
/* Enter main loop */
for (;;)
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 04985c9f91..448b5a4c35 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -1525,7 +1525,8 @@ ApplyWorkerMain(Datum main_arg)
/* Connect to our database. */
BackgroundWorkerInitializeConnectionByOid(MyLogicalRepWorker->dbid,
- MyLogicalRepWorker->userid);
+ MyLogicalRepWorker->userid,
+ 0);
/* Load the subscription into persistent memory context. */
ApplyContext = AllocSetContextCreate(TopMemoryContext,
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 6dc2095b9a..6b5a32b25b 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3774,7 +3774,7 @@ PostgresMain(int argc, char *argv[],
* it inside InitPostgres() instead. In particular, anything that
* involves database access should be there, not here.
*/
- InitPostgres(dbname, InvalidOid, username, InvalidOid, NULL);
+ InitPostgres(dbname, InvalidOid, username, InvalidOid, NULL, false);
/*
* If the PostmasterContext is still around, recycle the space; we don't
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 484628987f..83640e3159 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -66,7 +66,7 @@
static HeapTuple GetDatabaseTuple(const char *dbname);
static HeapTuple GetDatabaseTupleByOid(Oid dboid);
static void PerformAuthentication(Port *port);
-static void CheckMyDatabase(const char *name, bool am_superuser);
+static void CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connections);
static void InitCommunication(void);
static void ShutdownPostgres(int code, Datum arg);
static void StatementTimeoutHandler(void);
@@ -290,7 +290,7 @@ PerformAuthentication(Port *port)
* CheckMyDatabase -- fetch information from the pg_database entry for our DB
*/
static void
-CheckMyDatabase(const char *name, bool am_superuser)
+CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connections)
{
HeapTuple tup;
Form_pg_database dbform;
@@ -326,7 +326,7 @@ CheckMyDatabase(const char *name, bool am_superuser)
/*
* Check that the database is currently allowing connections.
*/
- if (!dbform->datallowconn)
+ if (!dbform->datallowconn && !override_allow_connections)
ereport(FATAL,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("database \"%s\" is not currently accepting connections",
@@ -563,7 +563,7 @@ BaseInit(void)
*/
void
InitPostgres(const char *in_dbname, Oid dboid, const char *username,
- Oid useroid, char *out_dbname)
+ Oid useroid, char *out_dbname, bool override_allow_connections)
{
bool bootstrap = IsBootstrapProcessingMode();
bool am_superuser;
@@ -1006,7 +1006,7 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
* user is a superuser, so the above stuff has to happen first.)
*/
if (!bootstrap)
- CheckMyDatabase(dbname, am_superuser);
+ CheckMyDatabase(dbname, am_superuser, override_allow_connections);
/*
* Now process any command-line switches and any additional GUC variable
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index a4574cd533..d06f71aceb 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -419,7 +419,7 @@ extern AuxProcType MyAuxProcType;
extern void pg_split_opts(char **argv, int *argcp, const char *optstr);
extern void InitializeMaxBackends(void);
extern void InitPostgres(const char *in_dbname, Oid dboid, const char *username,
- Oid useroid, char *out_dbname);
+ Oid useroid, char *out_dbname, bool override_allow_connections);
extern void BaseInit(void);
/* in utils/init/miscinit.c */
diff --git a/src/include/postmaster/bgworker.h b/src/include/postmaster/bgworker.h
index 0c04529f47..f1d08a3866 100644
--- a/src/include/postmaster/bgworker.h
+++ b/src/include/postmaster/bgworker.h
@@ -140,10 +140,13 @@ extern PGDLLIMPORT BackgroundWorker *MyBgworkerEntry;
* If dbname is NULL, connection is made to no specific database;
* only shared catalogs can be accessed.
*/
-extern void BackgroundWorkerInitializeConnection(const char *dbname, const char *username);
+extern void BackgroundWorkerInitializeConnection(const char *dbname, const char *username, uint32 flags);
/* Just like the above, but specifying database and user by OID. */
-extern void BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid);
+extern void BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, uint32 flags);
+
+/* Flags to BackgroundWorkerInitializeConnection et al */
+#define BGWORKER_BYPASS_ALLOWCONN 1
/* Block/unblock signals in a background worker process */
extern void BackgroundWorkerBlockSignals(void);
