Re: Providing catalog view to pg_hba.conf file - Patch submission

All,

* Tomas Vondra (tomas(dot)vondra(at)2ndquadrant(dot)com) wrote:
> On 28.1.2015 23:01, Jim Nasby wrote:
> > On 1/28/15 12:46 AM, Haribabu Kommi wrote:
> >>> >Also, what happens if someone reloads the config in the middle of
> >>> running
> >>> >the SRF?
> >> hba entries are reloaded only in postmaster process, not in every
> >> backend.
> >> So there shouldn't be any problem with config file reload. Am i
> >> missing something?
> >
> > Ahh, good point. That does raise another issue though... there might
> > well be some confusion about that. I think people are used to the
> > varieties of how settings come into effect that perhaps this isn't an
> > issue with pg_settings, but it's probably worth at least mentioning in
> > the docs for a pg_hba view.
>
> I share this fear, and it's the main problem I have with this patch.

Uh, yeah, agreed.

> Currently, the patch always does load_hba() every time pg_hba_settings
> is accessed, which loads the current contents of the config file into
> the backend process, but the postmaster still uses the original one.
> This makes it impossible to look at currently effective pg_hba.conf
> contents. Damn confusing, I guess.

Indeed. At a *minimum*, we'd need to have some way to say "what you're
seeing isn't what's actually being used".

> Also, I dare to say that having a system view that doesn't actually show
> the system state (but contents of a config file that may not be loaded
> yet), is wrong.

Right, we need to show what's currently active in PG-land, not just spit
back whatever the on-disk contents currently are.

> Granted, we don't modify pg_hba.conf all that often, and it's usually
> followed by a SIGHUP to reload the contents, so both the backend and
> postmaster should see the same stuff. But the cases when I'd use this
> pg_hba_settings view usually turned out to be 'forgot to SIGHUP', so
> this would not help, actually.

Agreed.

> What I can imagine is keeping the original list (parsed by postmaster,
> containing the effective pg_hba.conf contents), and keeping another list
> of 'current contents' within backends. And having a 'reload' flag for
> pg_hba_settings, determining which list to use.
>
> pg_hba_settings(reload=false) -> old list (from postmaster)
> pg_hba_settings(reload=true) -> new list (from backend)
>
> The pg_hba_settings view should use 'reload=false' (i.e. show effective
> contents of the hba).

I don't think we actually care what the "current contents" are from the
backend's point of view- after all, when does an individual backend ever
use the contents of pg_hba.conf after it's up and running? What would
make sense, to me at least, would be:

pg_hba_configured() -- spits back whatever the config file has
pg_hba_active() -- shows what the postmaster is using currently

Or something along those lines. Note that I'd want pg_hba_configured()
to throw an error if the config file can't be parsed (which would be
quite handy to make sure that you didn't goof up the config..). This,
combined with pg_reload_conf() would provide a good way to review
changes before putting them into place.

> The other feature that'd be cool to have is a debugging function on top
> of the view, i.e. a function pg_hba_check(host, ip, db, user, pwd)
> showing which hba rule matched. But that's certainly nontrivial.

I'm not sure that I see why, offhand, it'd be much more than trivial..

Thanks!

Stephen