Re: pending patch: Re: HS/SR and smart shutdown

On Wed, Mar 31, 2010 at 6:02 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On Wed, 2010-03-31 at 17:48 +0900, Fujii Masao wrote:
>> On Wed, Mar 31, 2010 at 5:00 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> > Please add some docs that a) explains what the patch does and b) notes
>> > any changes from behaviour in previous releases. ISTM this is a major
>> > change in behaviour.
>>
>> How about adding the following description into "17.5. Shutting Down
>> the Server"?
>>
>>     If the server is in recovery, it waits for all of the read only
>>     connections to be closed.
>
> You need to explain which mode you're talking about.

Smart Shutdown mode

> Adding minimal
> comments isn't my objective. We need good, useful documentation on every
> aspect that you add or change.

But the patch doesn't provide anything beyond:

>> If the server is in recovery, it waits for all of the read only
>> connections to be closed.

What additional explanation do you think is required?

>> And, where should the note be put in? AFAIK, the previous behavior
>> has not been documented anywhere.
>
>> > >From what I have seen, the comment about PM_WAIT_BACKENDS is incorrect.
>> > "backends might be waiting for the WAL record that conflicts with their
>> > queries to be replayed". Recovery sometimes waits for backends, but
>> > backends never wait for recovery.
>>
>> Really? As Heikki explained before, backends might wait for the lock
>> taken by the startup process.
>> http://archives.postgresql.org/pgsql-hackers/2010-01/msg02984.php
>
> Backends wait for locks, yes, but they could be waiting for user locks
> also. That is not "waiting for the WAL record", that concept does not
> exist.

How about the following change?

- * waiting for the WAL record that conflicts with their queries to be
- * replayed, recovery and replication need to remain until all read
+ * waiting until the startup process has released the lock that
+ * their queries are waiting for by replaying the WAL record,
+ * recovery and replication need to remain until all read

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center