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

On Wed, Apr 6, 2016 at 11:17 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Tue, Apr 5, 2016 at 11:40 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >>
> >> > 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.
> >>
> >> Because a backend has no chance to call SyncRepUpdateConfig() and
> >> parse the latest value of s_s_names if SyncRepUpdateConfig() is not
> >> called here. This means that pg_stat_replication may return the
> >> information
> >> based on the old value of s_s_names.
> >>
> >
> > Thats right, but without this patch also won't pg_stat_replication can show
> > old information? If no, why so?
>
> Without the patch, when s_s_names is changed and SIGHUP is sent,
> a backend calls ProcessConfigFile(), parse the configuration file and
> set the global variable SyncRepStandbyNames to the latest value of
> s_s_names. When pg_stat_replication is accessed, a backend calculates
> which standby is synchronous based on that latest value in SyncRepStandbyNames,
> and then displays the information of sync replication.
>
> With the patch, basically the same steps are executed when s_s_names is
> changed. But the difference is that, with the patch, SyncRepUpdateConfig()
> must be called after ProcessConfigFile() is called before the calculation of
> sync standbys. So I just added the call of SyncRepUpdateConfig() to
> pg_stat_get_wal_senders().
>

Then why to call it just in pg_stat_get_wal_senders(), isn't it better if we call it always after ProcessConfigFile() (after setting SyncRepStandbyNames)

> BTW, we can move SyncRepUpdateConfig() just after ProcessConfigFile()
> from pg_stat_get_wal_senders() and every backends always parse the value
> of s_s_names when the setting is changed.
>