Re: Making WAL receiver startup rely on GUC context for primary_conninfo and primary_slot_name

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Sergei Kornilov <sk(at)zsrv(dot)org>, Donald Dong <xdong(at)csumb(dot)edu>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Postgres hackers <pgsql-hackers(at)postgresql(dot)org>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, "simon(at)2ndquadrant(dot)com" <simon(at)2ndquadrant(dot)com>, "ams(at)2ndquadrant(dot)com" <ams(at)2ndquadrant(dot)com>, "masao(dot)fujii(at)gmail(dot)com" <masao(dot)fujii(at)gmail(dot)com>
Subject: Re: Making WAL receiver startup rely on GUC context for primary_conninfo and primary_slot_name
Date: 2019-01-17 00:54:16
Message-ID: 20190117005416.GA2036@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jan 16, 2019 at 11:02:48AM -0500, Robert Haas wrote:
> I think you have some valid points here, but I also think that the
> patch as written doesn't really seem to have any user-visible
> benefits. Sure, ready_to_display is a crock, but getting rid of it
> doesn't immediately help anybody. And on the flip side your patch
> might have bugs, in which case we'll be worse off. I'm not going to
> stand on my soapbox and say that committing this patch is a terrible
> idea, because as far as I can tell, it isn't. But it feels like it
> might be a commit for the sake of making a commit, and I can't get too
> excited about that either. Since the logic will have to be further
> revised anyway to make primary_conninfo PGC_SIGHUP, why not just wait
> until that's done and then commit all the changes together instead of
> guessing that this might make that easier?

You can say that about any patch which gets committed, even
refactoring patches have a risk of introducing bugs, and even subtle
ones understood only after release.

I was justifying the existence of this thread exactly for opposite
reasons. After reviewing the other patch switch to reload the
parameters we could do more, and spawning a new thread to attract the
correct audience looked rather right (it still does):
https://www.postgresql.org/message-id/20181212043208.GI17695@paquier.xyz

And this refactoring seemed to be justified as part of a different
thread. I don't mind dropping this patch and this thread and just go
back there, but that seems like taking steps backward, and what's
proposed here is a bit different than just making the parameters
reloadable. Still the refactoring looks justified to me for
correctness with the parameter handling.

Anyway, based on the opinions gathered until now, I don't mind just
dropping this patch and move on to the other thread, though we will
likely finish with the same discussion as what's done here :)

A patch on which two committers are expressing concerns about is as
good as going to the waste bin. That's of course fine by me.
--
Michael

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2019-01-17 01:12:26 Re: [HACKERS] generated columns
Previous Message Jamison, Kirk 2019-01-17 00:42:14 RE: reloption to prevent VACUUM from truncating empty pages at the end of relation