On Sat, Apr 23, 2016 at 5:20 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > > On Sat, Apr 23, 2016 at 7:44 PM, Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Wed, Apr 20, 2016 at 12:46 PM, Kyotaro HORIGUCHI > > <horiguchi.kyotaro@lab.ntt.co.jp> wrote: > >> > >> > >> assign_s_s_names causes SEGV when it is called without calling > >> check_s_s_names. I think that's not the case for this varialbe > >> because it is unresettable amid a session. It is very uneasy for > >> me but I don't see a proper means to reset > >> syncrep_parse_result. > >> > > > > Is it because syncrep_parse_result is not freed after creating a copy of it > > in assign_synchronous_standby_names()? If it so, then I think we need to > > call SyncRepFreeConfig(syncrep_parse_result); in > > assign_synchronous_standby_names at below place: > > > > + /* Copy the parsed config into TopMemoryContext if exists */ > > > > + if (syncrep_parse_result) > > > > + SyncRepConfig = SyncRepCopyConfig(syncrep_parse_result); > > > > Could you please explain how to trigger the scenario where you have seen > > SEGV? > > Seeing this discussion moving on, I am wondering if we should not > discuss those improvements for 9.7. >
The main point for this improvement is that the handling for guc s_s_names is not similar to what we do for other somewhat similar guc's and which causes in-efficiency in non-hot code path (less used code). So, we can push this improvement to 9.7, but OTOH we can also consider it as a non-beta blocker issue and see if we can make this code path better in the mean time.