Created attachment 177179 [details] suggested patch for rc.d script When restarting postgresql server via rc scripts, the server's process inherits the fib of the caller, ignoring any settings in /etc/rc.conf. During maintenance, this can cause headaches when simply restarting the database brings it up with the incorrect routing table. This is likely due to the rc script's use of 'su -l ${postgresql_user} -c "exec ${command} ..." in the script. A simple fix is to insert "setfib ${postgresql_fib}" in the exec statment ; when no fib is configured, this will default to 0 in any case without any error. This is provided by the patch below. Caveat: this would break port installations on any OS prior to a base where /usr/sbin/setfib is not in use (but this was tested and found present on amd64 versions as early as 7.2 – may still want to modify to account for early OS edge cases rather than calling setfib always implicitly)
This will break if no FIBs are used. See here for a working example (documenting the option and adding pre_cmd to modify $command): https://svnweb.freebsd.org/ports/head/devel/phabricator/files/phd.in?revision=366900&view=markup
Created attachment 177180 [details] More robust patch for using setfib (untested) @Ryder: Could you try this patch? (untested)
Should this feature/support be added to all postgresqlXX-server ports?
(In reply to Kubilay Kocak from comment #3) That would make sense. I also feel like that it would be better to get rid of postgresql_command() and use the standard functions from /etc/rc.subr for setting _user, _fib etc., but I'm not using the port myself and I didn't check if that would lead to ill side-effects, therefore this more conservative approach.
Thanks for catching my patch's error. It's why I shouldn't do this on Saturday mornings. Attempted your patch as-is against a fresh portsnap, but there was an error: $ sudo patch < ~/postrc.patch Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |--- files/postgresql.in.orig 2016-11-19 12:03:22.581531987 +0100 |+++ files/postgresql.in 2016-11-19 12:22:09.606409987 +0100 -------------------------- Patching file files/postgresql.in using Plan A... Hunk #1 succeeded at 14. Hunk #2 succeeded at 38. Hunk #3 failed at 64. Hunk #4 succeeded at 106. 1 out of 4 hunks failed--saving rejects to files/postgresql.in.rej done $ sudo cat files/postgresql.in.rej @@ -62,6 +64,7 @@ eval postgresql_data="\${postgresql_${profile}_data:-${postgresql_data}}" eval postgresql_flags="\${postgresql_${profile}_flags:-${postgresql_flags}}" eval postgresql_initdb_flags="\${postgresql_${profile}_initdb_flags:-${postgresql_initdb_flags}}" + eval postgresql_fib="\${postgresql_${profile}_fib:-${postgresql_fib}}" fi else if [ "x${postgresql_profiles}" != "x" -a "x$1" != "x" ]; then
Created attachment 177204 [details] Patch for all version of postgresqlXX-server, bump revision @ryder Please find attached a new version of the patch that covers all versions of postgresqlXX (patch was generated against r426569 of HEAD)
Checks out.
Any news here?
@w.schwarzenfeld@utanet.at Definitely still required @pgsql Is there anything you can do about it?
Sorry for leaving this one around for so long. What is special with PostgreSQL that makes it require special attention WRT FIBs? Is it the exec call? that should perhaps be changed rather than the suggested solution? Otherwise, shouldn't all daemons have some similar FIB support? Palle
It's about its own special calls of exec (combined with su), overloading of start/restart/..., providing a special initdb target and the fact that it supports profiles (also compare with www/apache24's handling of the matter). More simple daemons (those that don't replace start, restart etc.) are handled automatically by /etc/rc.subr, but if you role your own thing (overload commands and pass them into pg_ctl) you have to manage fibs yourself at the moment. One could think of adding some support in /etc/rc.subr that allows overloading of start etc. more easily, so one can my setting the user, fib etc. while still running their own commands. Also, profiles could become a standardized feature, but all of this is outside of the scope of this PR.
@Palle Any updates?
Hi, Perhaps the su -l ... -c 'exec ...' is not the best way to start postgresql either way. I'll review this and see if there is simpler way, or else commit this suggestion. Palle
(In reply to Palle Girgensohn from comment #13) Was this ever applied?
(In reply to Michael Gmelin from comment #14) No, sadly not. Thanks for reminding me. Palle
(In reply to Palle Girgensohn from comment #15) Any chance to fix this before the PRs third anniversary? ;)
ping