Postgres Pro
Downloads
Home   >   mailing lists

Re: Support for N synchronous standby servers - take 2 - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: Support for N synchronous standby servers - take 2
Date
Msg-id CAA4eK1L55ZgqZKFDQrtR7dnxuY+EYU3aPh+FV1avHgrxWierhg@mail.gmail.com
Whole thread Raw
In response to Re: Support for N synchronous standby servers - take 2  (Fujii Masao <masao.fujii@gmail.com>)
Responses Re: Support for N synchronous standby servers - take 2
List pgsql-hackers
Tree view
On Mon, Apr 4, 2016 at 1:58 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>
>
> Thanks for updating the patch!
>
> I applied the following changes to the patch.
> Attached is the revised version of the patch.
>

1.
       {
{"synchronous_standby_names", PGC_SIGHUP, REPLICATION_MASTER,
gettext_noop("List of names of potential synchronous standbys."),
NULL,
GUC_LIST_INPUT
},
&SyncRepStandbyNames,
"",
check_synchronous_standby_names, NULL, NULL
},

Isn't it better to modify the description of synchronous_standby_names in guc.c based on new usage?

2.
pg_stat_get_wal_senders()
{
..
/*
! * Allocate and update the config data of synchronous replication,
! * and then get the currently active synchronous standbys.
  */
+ SyncRepUpdateConfig();
  LWLockAcquire(SyncRepLock, LW_SHARED);
! sync_standbys = SyncRepGetSyncStandbys();
  LWLockRelease(SyncRepLock);
..
}

Why is it important to update the config with patch?  Earlier also any update to config between calls wouldn't have been visible.


3.
      <title>Planning for High Availability</title>
  
     <para>
!     <varname>synchronous_standby_names</> specifies the number of
!     synchronous standbys that transaction commits made when

Is it better to say like: <varname>synchronous_standby_names</> specifies the number and names of


4.
+ /*
+  * Return the list of sync standbys, or NIL if no sync standby is connected.
+  *
+  * If there are multiple standbys with the same priority,
+  * the first one found is considered as higher priority.

Here line indentation of second line can be improved.

5.
! /*
! * syncrep_yyparse sets the global syncrep_parse_result as side effect.
! * But this function is required to just check, so frees it
! * once parsing parameter.
! */
! SyncRepFreeConfig(syncrep_parse_result);

How about below change in comment?
/so frees it once parsing parameter/so frees it after parsing the parameter


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

pgsql-hackers by date:

Previous
From: Dilip Kumar
Date:
Subject: Re: Move PinBuffer and UnpinBuffer to atomics
Next
From: Simon Riggs
Date:
Subject: Re: Support for N synchronous standby servers - take 2