On Wed, Mar 29, 2017 at 5:04 PM, Tsunakawa, Takayuki
<tsunakawa.ta...@jp.fujitsu.com> wrote:
> From: pgsql-hackers-ow...@postgresql.org
>> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier
>> What do you think about the updated version attached?
> I reviewed this patch.  Here are some comments and questions:

Thanks!

> (1) monitoring.sgml
> The new row needs to be placed in the "Timeout" group.  The current patch 
> puts the row at the end of IO group.  Please insert the new row according to 
> the alphabetical order.
>
> In addition, "morerows" attribute of the following line appears to be 
> increased.
>
>          <entry morerows="2"><literal>Timeout</></entry>
>
> By the way, doesn't this wait event belong to IPC wait event type, because 
> the process is waiting for other conflicting processes to terminate the 
> conflict conditions?  Did you choose Timeout type because 
> max_standby_{archive | streaming}_delay relates to this wait?  I'm not 
> confident which is appropriate, but I'm afraid users can associate this wait 
> with a timeout.

Actually I think that this event belongs to the timeout category,
because we wait until the timeout has finished, the latch being here
to be sure that the process is more responsive in case of a postmaster
death.

> (2) standby.c
> Do we need to specify WL_LATCH_SET?  Who can set the latch?  Do the backends 
> who ended the conflict set the latch?

This makes the process able to react on SIGHUP. That's useful for
responsiveness.

> (3) standby.c
> +       if (rc & WL_LATCH_SET)
> +               ResetLatch(MyLatch);
> +
> +       /* emergency bailout if postmaster has died */
> +       if (rc & WL_POSTMASTER_DEATH)
> +               proc_exit(1);
>
> I thought the child processes have to terminate as soon as postmaster 
> vanishes.  So, it would be better for the order of the two if statements 
> above to be reversed.  proc_exit() could be exit(), as some children do in 
> postmaster/*.c.

OK, reversed this order.

Attached is v4.
-- 
Michael

Attachment: standby-delay-latch-v4.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to