Re: Re: [COMMITTERS] pgsql: Check compulsory parameters in recovery.conf in standby_mode, per - Mailing list pgsql-hackers

Simon Riggs wrote:
> On Tue, 2010-04-06 at 12:38 +0300, Heikki Linnakangas wrote:
>> Simon Riggs wrote:
>>> On Tue, 2010-04-06 at 10:19 +0300, Heikki Linnakangas wrote:
>>>> Fujii Masao wrote:
>>>>> On Tue, Apr 6, 2010 at 3:29 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>>>>>> I was also surprised to note that the Startup process is not signaled by
>>>>>> WALReceiver when new WAL is received, so it will continue sleeping even
>>>>>> though it has work to do.
>>>>> I don't think this is so useful in 9.0 since synchronous replication
>>>>> (i.e., transaction commit wait for WAL to be replayed by the standby)
>>>>> is not supported.
>>>> Well, it would still be useful, as it would shorten the delay. But yeah,
>>>> there's a delay in asynchronous replication anyway, so we decided to
>>>> keep it simple and just poll. It's not ideal and definitely needs to be
>>>> revisited for synchronous mode in the future. Same for walsenders.
>>> A signal seems fairly straightforward to me, the archiver did this in
>>> 8.0 and it was not considered complex then. Quite why it would be
>>> complex here is not clear.
>> The other side of the problem is that walsender polls too. Eliminating
>> the delay from walreceiver doesn't buy you much unless you eliminate the
>> delay from the walsender too. And things get complicated there. Do you
>> signal the walsenders at every commit? That would be a lot of volume,
>> and adds more work for every normal transaction in the primary. Maybe
>> not much, but it would be one more thing to worry about and test.
>
> You are trying to connect two unrelated things.
>
> We can argue that the WALSender's delay allows it to build up a good
> sized batch of work to transfer.
>
> Having the Startup process wait does not buy us anything at all.
> Currently if the Startup process finishes more quickly than the
> WALreceiver it will wait for 100ms.

Ok, here's a patch to add signaling between walreceiver and startup
process. It indeed isn't much code, and seems pretty safe, so if no-one
objects strongly, I'll commit. It won't help on platforms where
pg_usleep() isn't interrupted by signals, though, but we can live with that.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index abdf4d8..47cd6e9 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8682,6 +8682,16 @@ StartupProcShutdownHandler(SIGNAL_ARGS)
         shutdown_requested = true;
 }

+/*
+ * SIGUSR1: do nothing, but receiving the signal will wake us up if we're
+ * sleeping (on platforms where usleep() is interrupted by sleep, on other
+ * platforms this is useless).
+ */
+static void
+DoNothingHandler(SIGNAL_ARGS)
+{
+}
+
 /* Handle SIGHUP and SIGTERM signals of startup process */
 void
 HandleStartupProcInterrupts(void)
@@ -8735,7 +8745,7 @@ StartupProcessMain(void)
     else
         pqsignal(SIGALRM, SIG_IGN);
     pqsignal(SIGPIPE, SIG_IGN);
-    pqsignal(SIGUSR1, SIG_IGN);
+    pqsignal(SIGUSR1, DoNothingHandler);    /* WAL record has been streamed */
     pqsignal(SIGUSR2, SIG_IGN);

     /*
@@ -8859,8 +8869,10 @@ retry:
                         goto triggered;

                     /*
-                     * When streaming is active, we want to react quickly when
-                     * the next WAL record arrives, so sleep only a bit.
+                     * Sleep until the next WAL record arrives, or
+                     * walreceiver dies. On some platforms, signals don't
+                     * interrupt sleep, so poll every 100 ms to ensure we
+                     * respond promptly on such platforms.
                      */
                     pg_usleep(100000L); /* 100ms */
                 }
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 98ab484..39d8fb9 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -205,8 +205,8 @@ bool        enable_bonjour = false;
 char       *bonjour_name;

 /* PIDs of special child processes; 0 when not running */
-static pid_t StartupPID = 0,
-            BgWriterPID = 0,
+pid_t        StartupPID = 0;
+static pid_t BgWriterPID = 0,
             WalWriterPID = 0,
             WalReceiverPID = 0,
             AutoVacPID = 0,
@@ -433,6 +433,7 @@ typedef struct
     PMSignalData *PMSignalState;
     InheritableSocket pgStatSock;
     pid_t        PostmasterPid;
+    pid_t        StartupPid;
     TimestampTz PgStartTime;
     TimestampTz PgReloadTime;
     bool        redirection_done;
@@ -4595,6 +4596,7 @@ save_backend_variables(BackendParameters *param, Port *port,
         return false;

     param->PostmasterPid = PostmasterPid;
+    param->StartupPid = StartupPid;
     param->PgStartTime = PgStartTime;
     param->PgReloadTime = PgReloadTime;

@@ -4809,6 +4811,7 @@ restore_backend_variables(BackendParameters *param, Port *port)
     read_inheritable_socket(&pgStatSock, ¶m->pgStatSock);

     PostmasterPid = param->PostmasterPid;
+    StartupPid = param->StartupPid;
     PgStartTime = param->PgStartTime;
     PgReloadTime = param->PgReloadTime;

diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index c0e7e0b..c4b4614 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -41,6 +41,7 @@
 #include "access/xlog_internal.h"
 #include "libpq/pqsignal.h"
 #include "miscadmin.h"
+#include "postmaster/postmaster.h"
 #include "replication/walreceiver.h"
 #include "storage/ipc.h"
 #include "storage/pmsignal.h"
@@ -532,6 +533,9 @@ XLogWalRcvFlush(void)
         walrcv->receivedUpto = LogstreamResult.Flush;
         SpinLockRelease(&walrcv->mutex);

+        /* Wake up startup process to process the arrived records */
+        kill(StartupPID, SIGUSR1);
+
         /* Report XLOG streaming progress in PS display */
         snprintf(activitymsg, sizeof(activitymsg), "streaming %X/%X",
                  LogstreamResult.Write.xlogid, LogstreamResult.Write.xrecoff);
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 56d7d8e..fcfca78 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -36,6 +36,8 @@ extern HANDLE PostmasterHandle;

 extern const char *progname;

+extern pid_t StartupPID;
+
 extern int    PostmasterMain(int argc, char *argv[]);
 extern void ClosePostmasterPorts(bool am_syslogger);