Re: [HACKERS] Commits don't block for synchronous replication

On Thu, Nov 23, 2017 at 4:32 AM, Ashwin Agrawal <aagrawal(at)pivotal(dot)io> wrote:
>
> On Wed, Nov 22, 2017 at 9:57 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>>
>> On 15 November 2017 at 10:07, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
>> wrote:
>> > On Wed, Nov 15, 2017 at 7:28 AM, Ashwin Agrawal <aagrawal(at)pivotal(dot)io>
>> > wrote:
>> >>
>> >> https://commitfest.postgresql.org/15/1297/
>> >>
>> >> Am I missing something or not looking at right place, this is marked as
>> >> committed but don't see the change in latest master ?
>> >
>> > Good thing you double-checked. This has been marked as committed
>> > eleven day ago by Simon (added in CC), but no commit has happened. I
>> > am switching back the status as "ready for committer".
>>
>> The patch has been applied - look at the code. Marking back to committed.
>
> I have no idea which magical place this is being committed, atleast don't
> see on master unless checking something wrong, please can you post the
> commit here ?

I am afraid that I have to agree with Ashwin here, and would like to
know the commit number where you applied it.

The code on HEAD (and back-branches) in syncrep.c, does that, in
SyncRepWaitForLSN():
/*
* Fast exit if user has not requested sync replication, or there are no
* sync replication standby names defined. Note that those
standbys don't
* need to be connected.
*/
if (!SyncRepRequested() || !SyncStandbysDefined())
return;

And the change proposed by Ashwin & co to address what is a bug is that:
/*
- * Fast exit if user has not requested sync replication, or there are no
- * sync replication standby names defined. Note that those
standbys don't
- * need to be connected.
+ * Fast exit if user has not requested sync replication.
*/
- if (!SyncRepRequested() || !SyncStandbysDefined())
+ if (!SyncRepRequested())
return;

On top of that the last commit from a certain Simon Riggs on syncrep.c
is this one:
commit: e05f6f75dbe00a7349dccf1116b5ed983b4728c0
author: Simon Riggs <simon(at)2ndQuadrant(dot)com>
date: Fri, 12 Aug 2016 12:43:45 +0100
Code cleanup in SyncRepWaitForLSN()

This is older than the bug report of this thread. All those
indications point out that the patch has *not* been committed. So it
seems to me that you perhaps committed it to your local repository,
but forgot to push it to the remote. I am switching back the patch
status to what looks correct to me "Ready for committer". Thanks.
--
Michael