Re: Allow workers to override datallowconn

On 03/02/2018 12:16 PM, Magnus Hagander wrote:
> On Fri, Feb 23, 2018 at 7:55 PM, Magnus Hagander <magnus(at)hagander(dot)net
> <mailto:magnus(at)hagander(dot)net>> wrote:
>
> On Fri, Feb 23, 2018 at 7:52 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us
> <mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us>> wrote:
>
> Magnus Hagander <magnus(at)hagander(dot)net
> <mailto:magnus(at)hagander(dot)net>> writes:
> > Here's another attempt at moving this one forward. Basically this adds a
> > new GucSource being GUC_S_CLIENT_EARLY. It now runs through the parameters
> > once before CheckMyDatabase, with source set to GUC_S_CLIENT_EARLY. In this
> > source, *only* parameters that are flagged as GUC_ALLOW_EARLY will be set,
> > any other parameters are ignored (without error). For now, only the
> > ignore_connection_restriction is allowed at this stage. Then it runs
> > CheckMyDatabase(), and after that it runs through all the parameters again,
> > now with the GUC_S_CLIENT source as usual, which will now process all
> > other  variables.
>
> Ick.  This is an improvement over the other way of attacking the
> problem?
> I do not think so.
>
>
> Nope, I'm far from sure that it is. I just wanted to show what it'd
> look like.
>
> I personally think the second patch (the one adding a parameter to
> BackendWorkerInitializeConnection) is the cleanest one. It doesn't
> solve Andres' problem, but perhaps that should be the job of a
> different patch.
>
>
>
> FWIW, I just realized that thue poc patch that adds the GUC also breaks
> a large part of the regression tests. As a side-effect of it breaking
> how DateStyle works. That's obviously a fixable problem, but it seems
> not worth spending time on if that's not the way forward anyway.
>
> Andres, do you have any other ideas of directions to look that would
> make you withdraw your objection? I'm happy to try to write up a patch
> that solves it in a way that everybody can agree with. But right now it
> seems to be stuck between one way that's strongly objected to by you and
> one way that's strongly objected to by Tom. And I'd rather not have that
> end up with not getting the problem solved at all for *any* of the
> usecases...
>

My 2c: I think we should just go with the simplest solution, that is the
patch sent on 22/2 19:54 (i.e. BackgroundWorkerInitializeConnectionByOid
with an extra parameter).

It would be nice to have something more generic that also works for the
Andres' use case, but the patches submitted to this thread are not
particularly pretty. Also, Tom suggested there may be safety issues when
the GUCs are processed earlier - I agree Andres is right the GUCs are
effectively ASCII-only so the encoding is not an issue, but perhaps
there are other issues (Tom suggested this merely as an example).

So I agree with Magnus, the extra flag seems to be perfectly fine for
bgworkers, and I'd leave the more generic solution for a future patch if
anyone wants to hack on it.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services