From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: masao.fujii@oss.nttdata.com
Cc: pgsql-hackers@postgresql.org
Subject: Re: RecoveryWalAll and RecoveryWalStream wait events
Date: Tue, 18 Feb 2020 14:20:16 +0900 (JST)
Message-ID: <20200218.142016.2107516686265665330.horikyota.ntt@gmail.com> (raw)
In-Reply-To: <124997ee-096a-5d09-d8da-2c7a57d0816e@oss.nttdata.com>
References: <124997ee-096a-5d09-d8da-2c7a57d0816e@oss.nttdata.com>
Hello.
At Tue, 18 Feb 2020 12:25:51 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
> Hi,
>
> RecoveryWalAll and RecoveryWalStream wait events are documented as
> follows.
>
> RecoveryWalAll
> Waiting for WAL from any kind of source (local, archive or stream) at
> recovery.
>
> RecoveryWalStream
> Waiting for WAL from a stream at recovery.
>
> But as far as I read the code, RecoveryWalAll is reported only when
> waiting
> for WAL from a stream. So the current description looks
> incorrect. What's
> described now for RecoveryWalStream seems rather fit to
> RecoveryWalAll.
> I'd like to change the description of RecoveryWalAll to "Waiting for
> WAL
> from a stream at recovery".
Good catch!
> Regarding RecoveryWalStream, as far as I read the code, while this
> event is
> being reported, the startup process is waiting for next trial to
> retrieve
> WAL data when WAL data is not available from any sources, based on
> wal_retrieve_retry_interval. So this current description looks also
> incorrect. I'd like to change it to "Waiting when WAL data is not
> available
> from any kind of sources (local, archive or stream) before trying
> again
> to retrieve WAL data".
>
> Thought?
I agree that the corrected description sound correct in meaning. The
latter seems a bit lengthy, though.
> Also the current names of these wait events sound confusing. I think
> that RecoveryWalAll should be changed to RecoveryWalStream.
> RecoveryWalStream should be RecoveryRetrieveRetryInterval or
> something.
I agree to the former, I think RecoveryWalInterval works well enough.
> Another problem is that the current wait event types of them also look
> strange. Currently the type of them is Activity, but IMO it's better
> to
> use IPC for RecoveryWalAll because it's waiting for walreceiver to
> receive new WAL. Also it's better to use Timeout for RecoveryWalStream
> because it's waiting depending on wal_retrieve_retry_interval.
Do you mean condition variable by the "IPC"? But the WaitLatch waits
not only for new WAL but also for trigger, SIGHUP, shutdown and
walreceiver events other than new WAL. I'm not sure that condition
variable fits for the purpose.
> The changes of wait event types and names would break the
> compatibility
> of wait events in pg_stat_activity. So this change should not be
> applied
> to the back branches, but it's ok to apply in the master. Right?
FWIW, It seems right.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: horikyota.ntt@gmail.com, masao.fujii@oss.nttdata.com
Subject: Re: RecoveryWalAll and RecoveryWalStream wait events
In-Reply-To: <20200218.142016.2107516686265665330.horikyota.ntt@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox