Re: [PATCH] HINT: pg_hba.conf changed since last config reload

On 10/16/2014 11:34 PM, Craig Ringer wrote:
>
>
> Given the generally positive reception to this, here's a patch.
>
> The first patch adds an errhint_log , akin to the current errdetail_log,
> so we can send a different HINT to the server log than we do to the client.

The patch behaves as you describe.    I feel that this feature would be 
useful , and you implemented the suggestions given that requested the 
reload notice but be sent to the client but instead just a hint about 
checking the server log.

You follow the pattern set with detail_log which makes sense.  The 
variable name "hint_log" doesn't make it obvious to me that
the hint goes to the server log, but not the client.  The comment for 
errhint_log should maybe explicitly say that.


One question about the code:

Does errfinish (elog.c at around line 505) need to free hint_log  ? (I 
would assume it does)

Other than that the patch looks good to me.


---------

Something else I noticed while testing.  This isn't introduced by your 
patch but I am wondering if it an existing bug if I setup my 
configuration like this:


#data_directory = 'ConfigDir'           # use data in another directory                                        #
(changerequires restart)
 
hba_file = 'ConfigDir/pg_hba2.conf'     # host-based authentication file

and start postgres like

./postgres -D ../data

it looks for pg2hba2.conf at bin/ConfigDir/pg_hba2.conf   (relative to 
the bin directory I started it from)

Then if I change my pg_hba.conf and do a reload I get the following in 
the log

LOG:  parameter "hba_file" cannot be changed without restarting the server
LOG:  configuration file 
"/usr/local/pgsql95git/bin/../data/postgresql.conf" contains errors; 
unaffected changes were applied


set_config_option is comparing the relative path with the absolute path.

Steve


> (Even if DETAIL was appropriate for this info, which it isn't, I can't
> use errdetail_log because it's already used for other information in
> some of the same error sites.)
>
> The second patch adds a test during errors to report if pg_hba.conf is
> stale, or if pg_ident.conf is stale.
>
>
> Typical output, client:
>
> psql: FATAL:  Peer authentication failed for user "fred"
> HINT:  See the server error log for additional information.
>
>
> Typical output, server:
>
> LOG:  provided user name (fred) and authenticated user name (craig) do
> not match
> FATAL:  Peer authentication failed for user "fred"
> DETAIL:  Connection matched pg_hba.conf line 84: "local   all
>    all                                     peer"
> HINT:  pg_hba.conf has been changed since last server configuration
> reload. Reload the server configuration to apply the changes.
>
>
>
> I've added this to the next CF.
>
>
>
>