On Mon, Mar 9, 2020 at 8:06 PM Andres Freund <and...@anarazel.de> wrote:
>
> Hi,
>
> On 2020-03-09 18:40:32 -0400, James Coleman wrote:
> > On Mon, Mar 9, 2020 at 6:28 PM Andres Freund <and...@anarazel.de> wrote:
> > > > I wanted to get some initial feedback on the idea before writing a
> > > > patch:
> > > > does that seem like a reasonable change? Is it actually plausible to
> > > > distinguish between this state and "still recovering" (i.e., when
> > > > starting
> > > > up a hot standby but initial recovery hasn't completed so it
> > > > legitimately
> > > > can't accept connections yet)? If so, should we include the possibility
> > > > if
> > > > hot_standby isn't on, just in case?
> > >
> > > Yes, it is feasible to distinguish those cases. And we should, if we're
> > > going to change things around.
> >
> > I'll look into this hopefully soon, but it's helpful to know that it's
> > possible. Is it basically along the lines of checking to see if the
> > LSN is past the minimum recovery point?
>
> No, I don't think that's the right approach. IIRC the startup process
> (i.e. the one doing the WAL replay) signals postmaster once consistency
> has been achieved. So you can just use that state.
I've taken that approach in the attached patch (I'd expected to wait
until later to work on this...but it seemed pretty small so I ended up
hacking on it this evening).
I don't have tests included: I tried intentionally breaking the
existing behavior (returning no error when hot_standby=off), but
running make check-world (including tap tests) didn't find any
breakages. I can look into that more deeply at some point, but if you
happen to know a place we test similar things, then I'd be happy to
hear it.
One other question: how is error message translation handled? I
haven't added entries to the relevant files, but also I'm obviously
not qualified to write them.
Thanks,
James
From 0389f9dd7b48b3058849960826a49079abb58e58 Mon Sep 17 00:00:00 2001
From: James Coleman <jtc...@gmail.com>
Date: Mon, 9 Mar 2020 21:45:15 -0400
Subject: [PATCH v1] Improve standby connection denied error message.
Currently when a standby is finished starting up but hot_standby is
configured to off, the error message when a client connects is "the
database system is starting up", which is needless confusing (and not
really all that accurate either).
Instead send a helpful error message so that the user immediately knows
that their server is configured to deny these connections.
---
src/backend/postmaster/postmaster.c | 12 +++++++++---
src/include/libpq/libpq-be.h | 7 ++++++-
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 55187eb910..63258287f7 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -2287,6 +2287,12 @@ retry1:
(errcode(ERRCODE_CANNOT_CONNECT_NOW),
errmsg("the database system is starting up")));
break;
+ case CAC_STANDBY:
+ ereport(FATAL,
+ (errcode(ERRCODE_CANNOT_CONNECT_NOW),
+ errmsg("the database system is up, but hot_standby is off")));
+ break;
+
case CAC_SHUTDOWN:
ereport(FATAL,
(errcode(ERRCODE_CANNOT_CONNECT_NOW),
@@ -2436,10 +2442,10 @@ canAcceptConnections(int backend_type)
result = CAC_WAITBACKUP; /* allow superusers only */
else if (Shutdown > NoShutdown)
return CAC_SHUTDOWN; /* shutdown is pending */
- else if (!FatalError &&
- (pmState == PM_STARTUP ||
- pmState == PM_RECOVERY))
+ else if (!FatalError && pmState == PM_STARTUP)
return CAC_STARTUP; /* normal startup */
+ else if (!FatalError && pmState == PM_RECOVERY)
+ return CAC_STANDBY; /* connection disallowed on non-hot standby */
else if (!FatalError &&
pmState == PM_HOT_STANDBY)
result = CAC_OK; /* connection OK during hot standby */
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 82e57afc64..effa78a84a 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -70,7 +70,12 @@ typedef struct
typedef enum CAC_state
{
- CAC_OK, CAC_STARTUP, CAC_SHUTDOWN, CAC_RECOVERY, CAC_TOOMANY,
+ CAC_OK,
+ CAC_STARTUP,
+ CAC_SHUTDOWN,
+ CAC_RECOVERY,
+ CAC_STANDBY,
+ CAC_TOOMANY,
CAC_WAITBACKUP
} CAC_state;
base-commit: 71e0d0a73773b3985db658d3c5366ce5ceef76ae
--
2.17.1
