Le jeudi 02 juin 2005 à 13:35 +0200, Martin Pitt a écrit : > [EMAIL PROTECTED] [2005-06-01 20:17 +0200]: > > pg_ctrlcluster get the postmaster uid and gid from the cluster. > > It was 31:32 when i created it. > > > > I had deleted and recreated the postgres account to update it > > (you removed its shell and maybe other things so not to miss any > > part i used the postinst adduser command to recreate it). > > The new uid/gid where (fake ones) 122:122. > > D'oh, please don't do such things. System account IDs must not be > modified since that breaks all files that are owned by them. You can > modify all the other properties (GECOS, shell, whatever) without any > problem, but if you change the IDs, you also have to change the owner > and group of all files which are owned by the account, and > additionally stop all running servers before (they also run under the > postgres account).
> > Thus running the postgresql-7.4 init script fired pg_ctrcluster > > which retrieved the old uid/gid from the cluster (31:32) and > > changed the process uid/gid to match the cluster one before > > calling pg_ctl. > > pg_ctl was running with an inexistant account. > > Yes, this can lead to all sorts of weirdnesses. I will add a test to > pg_ctlcluster that will refuse to start the postmaster if the account > is not valid to prevent such failures in the future. That s way better than my hack . I also though it would be great to manage it at a higher level (less bloat then adding check in every part of the code that deal with uid, better do it at the entry point). Though i still have a comment ... > > And in pg_ctl , l.114 : > > if [ `$PGPATH/pg_id -u` -eq 0 ] > > then > > echo "$CMDNAME: cannot be run as root" 1>&2 > > (...) > > Right, this could be made much more robust with > > if [ "`$PGPATH/pg_id -u`" = 0 ] == would be posix complient. Though i am against the use for test ([]) when testing a command output (instead of a variable). I am addicted to : if `$PGPATH/pg_id -u` ; or use of a temp variable to store the command output. Easier to debug :) (it is pretty hard to debug command output in "if [ `command` ]" ) > > > echo "This user does not exists . You may have changed the uid > > of the account used to create this database cluster. You should > > dump, drop and reinit a the cluster." > > fi > > I will implement something like that in pg_ctlcluster in Perl. > > > Maybe all this is too much for a special case which may not > > appear often as the problem never arise as far as i searched for > > this error. Please close this bug if you think so. > > Well, I will close it when I added the check to make the error more > obvious. I am used to always putting quote around shell variables though this time i found it great you did not. Else i would never have found out why pg_ctl acted in a weird way. At least pg_ctl fails if it runs with a broken uid. With the quote it would not. I feel that the fact this code break if the process uid is invalid is a "good thing" ... with quote it will think that ".../pg_id: Success" in sterr is a valid uid when "0" is not. Maybe the check at pg_ctlcluster level is enough :) Thank you for the quick followup. Alban
