On Wed, Jul 29, 2020 at 11:24 AM Fujii Masao
<masao.fujii@oss.nttdata.com> wrote:
>
>
>
> On 2020/04/03 22:49, James Coleman wrote:
> > On Thu, Apr 2, 2020 at 5:53 PM David Zhang <david.zhang@highgo.ca> wrote:
> >>
> >> The following review has been posted through the commitfest application:
> >> make installcheck-world: not tested
> >> Implements feature: tested, passed
> >> Spec compliant: not tested
> >> Documentation: not tested
> >>
> >> I applied the patch to the latest master branch and run a test below. The error messages have been separated.
Belowis the test steps.
> >>
> >> ### setup primary server
> >> initdb -D /tmp/primary/data
> >> mkdir /tmp/archive_dir
> >> echo "archive_mode='on'" >> /tmp/primary/data/postgresql.conf
> >> echo "archive_command='cp %p /tmp/archive_dir/%f'" >> /tmp/primary/data/postgresql.conf
> >> pg_ctl -D /tmp/primary/data -l /tmp/primary-logs start
> >>
> >> ### setup host standby server
> >> pg_basebackup -p 5432 -w -R -D /tmp/hotstandby
> >> echo "primary_conninfo='host=127.0.0.1 port=5432 user=pgdev'" >> /tmp/hotstandby/postgresql.conf
> >> echo "restore_command='cp /tmp/archive_dir/%f %p'" >> /tmp/hotstandby/postgresql.conf
> >> echo "hot_standby = off" >> /tmp/hotstandby/postgresql.conf
> >> pg_ctl -D /tmp/hotstandby -l /tmp/hotstandby-logs -o "-p 5433" start
> >>
> >> ### keep trying to connect to hot standby server in order to get the error messages in different stages.
> >> while true; do echo "`date`"; psql postgres -p 5433 -c "SELECT txid_current_snapshot();" sleep 0.2; done
> >>
> >> ### before the patch
> >> psql: error: could not connect to server: FATAL: the database system is starting up
> >> ...
> >>
> >> ### after the patch, got different messages, one message indicates hot_standby is off
> >> psql: error: could not connect to server: FATAL: the database system is starting up
> >> ...
> >> psql: error: could not connect to server: FATAL: the database system is up, but hot_standby is off
> >> ...
> >
> > Thanks for the review and testing!
>
> Thanks for the patch! Here is the comment from me.
>
> + else if (!FatalError && pmState == PM_RECOVERY)
> + return CAC_STANDBY; /* connection disallowed on non-hot standby */
>
> Even if hot_standby is enabled, pmState seems to indicate PM_RECOVERY
> until recovery has reached a consistent state. No? So, if my understanding
> is right, "FATAL: the database system is up, but hot_standby is off" can
> be logged even when hot_standby is on. Which sounds very confusing.
That's a good point. I've attached a corrected version.
I still don't have a good idea for how to add a test for this change.
If a test for this warranted, I'd be interested in any ideas.
James