Skip to content

Commit 6e1dd27

Browse files
anarazelanarazel
committed
Unify SIGHUP handling between normal and walsender backends.
Because walsender and normal backends share the same main loop it's problematic to have two different flag variables, set in signal handlers, indicating a pending configuration reload. Only certain walsender commands reach code paths checking for the variable (START_[LOGICAL_]REPLICATION, CREATE_REPLICATION_SLOT ... LOGICAL, notably not base backups). This is a bug present since the introduction of walsender, but has gotten worse in releases since then which allow walsender to do more. A later patch, not slated for v10, will similarly unify SIGHUP handling in other types of processes as well. Author: Petr Jelinek, Andres Freund Reviewed-By: Michael Paquier Discussion: https://postgr.es/m/20170423235941.qosiuoyqprq4nu7v@alap3.anarazel.de Backpatch: 9.2-, bug is present since 9.0
1 parent c6c3334 commit 6e1dd27

File tree

4 files changed

+27
-38
lines changed

4 files changed

+27
-38
lines changed

‎src/backend/replication/walsender.c

+7-22
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,6 @@ static bool streamingDoneReceiving;
182182
static bool WalSndCaughtUp = false;
183183

184184
/* Flags set by signal handlers for later service in main loop */
185-
static volatile sig_atomic_t got_SIGHUP = false;
186185
static volatile sig_atomic_t got_SIGUSR2 = false;
187186
static volatile sig_atomic_t got_STOPPING = false;
188187

@@ -218,7 +217,6 @@ static struct
218217
} LagTracker;
219218

220219
/* Signal handlers */
221-
static void WalSndSigHupHandler(SIGNAL_ARGS);
222220
static void WalSndLastCycleHandler(SIGNAL_ARGS);
223221

224222
/* Prototypes for private functions */
@@ -1201,9 +1199,9 @@ WalSndWriteData(LogicalDecodingContext *ctx, XLogRecPtr lsn, TransactionId xid,
12011199
CHECK_FOR_INTERRUPTS();
12021200

12031201
/* Process any requests or signals received recently */
1204-
if (got_SIGHUP)
1202+
if (ConfigReloadPending)
12051203
{
1206-
got_SIGHUP = false;
1204+
ConfigReloadPending = false;
12071205
ProcessConfigFile(PGC_SIGHUP);
12081206
SyncRepInitConfig();
12091207
}
@@ -1309,9 +1307,9 @@ WalSndWaitForWal(XLogRecPtr loc)
13091307
CHECK_FOR_INTERRUPTS();
13101308

13111309
/* Process any requests or signals received recently */
1312-
if (got_SIGHUP)
1310+
if (ConfigReloadPending)
13131311
{
1314-
got_SIGHUP = false;
1312+
ConfigReloadPending = false;
13151313
ProcessConfigFile(PGC_SIGHUP);
13161314
SyncRepInitConfig();
13171315
}
@@ -2101,9 +2099,9 @@ WalSndLoop(WalSndSendDataCallback send_data)
21012099
CHECK_FOR_INTERRUPTS();
21022100

21032101
/* Process any requests or signals received recently */
2104-
if (got_SIGHUP)
2102+
if (ConfigReloadPending)
21052103
{
2106-
got_SIGHUP = false;
2104+
ConfigReloadPending = false;
21072105
ProcessConfigFile(PGC_SIGHUP);
21082106
SyncRepInitConfig();
21092107
}
@@ -2908,19 +2906,6 @@ HandleWalSndInitStopping(void)
29082906
got_STOPPING = true;
29092907
}
29102908

2911-
/* SIGHUP: set flag to re-read config file at next convenient time */
2912-
static void
2913-
WalSndSigHupHandler(SIGNAL_ARGS)
2914-
{
2915-
int save_errno = errno;
2916-
2917-
got_SIGHUP = true;
2918-
2919-
SetLatch(MyLatch);
2920-
2921-
errno = save_errno;
2922-
}
2923-
29242909
/*
29252910
* SIGUSR2: set flag to do a last cycle and shut down afterwards. The WAL
29262911
* sender should already have been switched to WALSNDSTATE_STOPPING at
@@ -2942,7 +2927,7 @@ void
29422927
WalSndSignals(void)
29432928
{
29442929
/* Set up signal handlers */
2945-
pqsignal(SIGHUP, WalSndSigHupHandler); /* set flag to read config
2930+
pqsignal(SIGHUP, PostgresSigHupHandler); /* set flag to read config
29462931
* file */
29472932
pqsignal(SIGINT, SIG_IGN); /* not used */
29482933
pqsignal(SIGTERM, die); /* request shutdown */

‎src/backend/tcop/postgres.c

+14-16
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,6 @@ char *stack_base_ptr = NULL;
122122
char *register_stack_base_ptr = NULL;
123123
#endif
124124

125-
/*
126-
* Flag to mark SIGHUP. Whenever the main loop comes around it
127-
* will reread the configuration file. (Better than doing the
128-
* reading in the signal handler, ey?)
129-
*/
130-
static volatile sig_atomic_t got_SIGHUP = false;
131-
132125
/*
133126
* Flag to keep track of whether we have started a transaction.
134127
* For extended query protocol this has to be remembered across messages.
@@ -187,7 +180,6 @@ static bool IsTransactionExitStmt(Node *parsetree);
187180
static bool IsTransactionExitStmtList(List *pstmts);
188181
static bool IsTransactionStmtList(List *pstmts);
189182
static void drop_unnamed_stmt(void);
190-
static void SigHupHandler(SIGNAL_ARGS);
191183
static void log_disconnections(int code, Datum arg);
192184

193185

@@ -2684,13 +2676,19 @@ FloatExceptionHandler(SIGNAL_ARGS)
26842676
"invalid operation, such as division by zero.")));
26852677
}
26862678

2687-
/* SIGHUP: set flag to re-read config file at next convenient time */
2688-
static void
2689-
SigHupHandler(SIGNAL_ARGS)
2679+
/*
2680+
* SIGHUP: set flag to re-read config file at next convenient time.
2681+
*
2682+
* Sets the ConfigReloadPending flag, which should be checked at convenient
2683+
* places inside main loops. (Better than doing the reading in the signal
2684+
* handler, ey?)
2685+
*/
2686+
void
2687+
PostgresSigHupHandler(SIGNAL_ARGS)
26902688
{
26912689
int save_errno = errno;
26922690

2693-
got_SIGHUP = true;
2691+
ConfigReloadPending = true;
26942692
SetLatch(MyLatch);
26952693

26962694
errno = save_errno;
@@ -3632,8 +3630,8 @@ PostgresMain(int argc, char *argv[],
36323630
WalSndSignals();
36333631
else
36343632
{
3635-
pqsignal(SIGHUP, SigHupHandler); /* set flag to read config
3636-
* file */
3633+
pqsignal(SIGHUP, PostgresSigHupHandler); /* set flag to read config
3634+
* file */
36373635
pqsignal(SIGINT, StatementCancelHandler); /* cancel current query */
36383636
pqsignal(SIGTERM, die); /* cancel current query and exit */
36393637

@@ -4046,9 +4044,9 @@ PostgresMain(int argc, char *argv[],
40464044
* (6) check for any other interesting events that happened while we
40474045
* slept.
40484046
*/
4049-
if (got_SIGHUP)
4047+
if (ConfigReloadPending)
40504048
{
4051-
got_SIGHUP = false;
4049+
ConfigReloadPending = false;
40524050
ProcessConfigFile(PGC_SIGHUP);
40534051
}
40544052

‎src/backend/utils/init/globals.c

+1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ volatile bool QueryCancelPending = false;
3131
volatile bool ProcDiePending = false;
3232
volatile bool ClientConnectionLost = false;
3333
volatile bool IdleInTransactionSessionTimeoutPending = false;
34+
volatile sig_atomic_t ConfigReloadPending = false;
3435
volatile uint32 InterruptHoldoffCount = 0;
3536
volatile uint32 QueryCancelHoldoffCount = 0;
3637
volatile uint32 CritSectionCount = 0;

‎src/include/miscadmin.h

+5
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
#ifndef MISCADMIN_H
2424
#define MISCADMIN_H
2525

26+
#include <signal.h>
27+
2628
#include "pgtime.h" /* for pg_time_t */
2729

2830

@@ -81,6 +83,7 @@ extern PGDLLIMPORT volatile bool InterruptPending;
8183
extern PGDLLIMPORT volatile bool QueryCancelPending;
8284
extern PGDLLIMPORT volatile bool ProcDiePending;
8385
extern PGDLLIMPORT volatile bool IdleInTransactionSessionTimeoutPending;
86+
extern PGDLLIMPORT volatile sig_atomic_t ConfigReloadPending;
8487

8588
extern volatile bool ClientConnectionLost;
8689

@@ -273,6 +276,8 @@ extern void restore_stack_base(pg_stack_base_t base);
273276
extern void check_stack_depth(void);
274277
extern bool stack_is_too_deep(void);
275278

279+
extern void PostgresSigHupHandler(SIGNAL_ARGS);
280+
276281
/* in tcop/utility.c */
277282
extern void PreventCommandIfReadOnly(const char *cmdname);
278283
extern void PreventCommandIfParallelMode(const char *cmdname);

0 commit comments

Comments
 (0)