On Mon, Apr 4, 2016 at 1:58 PM, Fujii Masao <masao.fu...@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
