2015-02-27 17:59 GMT+01:00 Stephen Frost <sfr...@snowman.net>: > All, > > * Tomas Vondra (tomas.von...@2ndquadrant.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. >
yes, good notice. I was blind. > > > 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 > I disagree and I dislike this direction. It is looks like over engineering. * load every time is wrong, because you will see possibly not active data. * ignore reload is a attack to mental health of our users. It should to work like "pg_settings". I need to see "what is wrong in this moment" in pg_hba.conf, not what was or what will be wrong. We can load any config files via admin contrib module - so there is not necessary repeat same functionality Regards Pavel > 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 >