On Wed, Apr 6, 2016 at 11:17 AM, Fujii Masao <masao.fu...@gmail.com> wrote: > > On Tue, Apr 5, 2016 at 11:40 PM, Amit Kapila <amit.kapil...@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. > That sounds appropriate, but not sure what is exact place to call it. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com