Re: Linux LSB init script - Mailing list pgsql-hackers

Peter Eisentraut <peter_e@gmx.net> wrote:
> OK, I have a number of comments on this proposal.
Thanks for looking at it.
> What does it buy us, in general, and compared to the existing
> (non-LSB) script?
A conforming script for use in LSB conforming implementations.
> what LSB actually entails, which is:
> 
> - INFO header
> - standardized exit codes
> - functions from /lib/lsb/init-functions
As well as a standardized set of actions and standard semantics for
them, if we're only talking about the init script parts.
> My argument is that it might be better to just merge the INFO header
> and the exit codes into the existing script and forget about using
> the functions.
That's where I started, but felt that it was of no benefit unless the
OS used them, in which case I thought conforming behavior would be
wanted.
> The old script already has a chkconfig header, which was the moral
> predecessor to the LSB INFO section. So someone evidently thought
> this sort of thing was useful, and so we might as well update it.
I'm not following what you say here.  Is there something wrong with
the current chkconfig header in the non-LSB script?
> Differentiating the exit codes is the bulk of your code, but what
> about merging this part into pg_ctl itself?
That would result in different exit codes for a lot of circumstances. 
I'm OK with that, if everyone agrees that we want, for example an exit
code of 0 for an attempt to start a server which is already running
or an attempt to stop a server which isn't running.  (These are only
two examples of differences between what is required of an LSB
conforming init script and what pg_ctl does.  Are we all in universal
agreement to make such a change to pg_ctl?
> The init-functions then don't really buy you anything, except that
> you replace pg_ctl with start_daemon and killproc, and good old echo
> with log_failure_msg.  And then you proceed in pg_initd_stop to
> basically reimplement half of the pg_ctl logic in shell code.
Well, with differences in behavior, of course, like attempting a fast
shutdown, and escalating to an immediate shutdown if that doesn't
succeed in the allowed time.
> And using the init-functions is the only thing here that is not
> backward compatible, so removing it would address the concern about
> support pre/non-LSB distributions.
Use of these functions is a requirement for LSB conforming scripts. 
If you don't use them, particularly for success and failure messages,
you're not conforming.  For one thing, using these functions causes
the success and failure messages to be formatted like those for other
services on the machine, and the format varies a lot between
distributions.  Also, consider this statement in the spec:
| Error and status messages should be printed with the logging
| functions (see Init Script Functions) log_success_msg(),
| log_failure_msg() and log_warning_msg(). Scripts may write to
| standard error or standard output, but implementations need not
| present text written to standard error/output to the user or do
| anything else with it.
Not using the functions would cause the script to only work for some
undefined subset of conforming implementations.
> First, the is, in my mind, no need to repeat half of the LSB spec in
> the comments.
Well, that's a bit of hyperbole -- I did the math and it's less than
10% of what I consider to be the part of the spec fairly directly
related to init scripts.
Which parts of these comments do you feel should be omitted?:
# Actions other than status may use these return codes:
#  1 - generic or unspecified error
#  2 - invalid or excess argument(s)
#  3 - unimplemented feature (for example, "reload")
#  4 - user had insufficient privilege
#  5 - program is not installed
#  6 - program is not configured
#  7 - program is not running
   # Start the service.   # If already running, return success without start attempt.
   # Stop the service.   # If not running, return success without stop attempt.
   # Stop and restart the service if the service is already running,   # otherwise start the service.
   # Restart the service if the service is already running.   # If stopped, return success without stop attempt.
   # Cause the configuration of the service to be reloaded   # without actually stopping and restarting the service.
#(Since PostgreSQL supports reload, force-reload should use
 
that.)
   # Print the current status of the service.   # Return codes differ from other actions:   #  0 - program is running
orservice is OK   #  1 - program is dead and /var/run pid file exists   #  2 - program is dead and /var/lock lock file
exists  #  3 - program is not running   #  4 - program or service status is unknown
 
   # If we don't recognize action, consider it an invalid argument.   # If the standard adds actions we don't support,
exitshould be 3
 
for those.
I put in as much as I found useful to myself while working on the
script.  I left it in on the assumption that it might also help
someone reading it.
> Second, there is no need to check for the existence of every script,
> program, file, and command line argument separately.  The existing
> script actually does that well enough.  If something is missing, the
> shell itself will complain soon enough.
But not with the exit code which LSB requires.  I don't think the
script complies with the spec as well if you rip out the checks.  You
could also get weird behavior, like a success code and nothing done
when things actually fail.   (I believe you have now identified a
place where the non-LSB script could use improvement.)
> Quite aside from anything else, I'm fairly sure a bloated script
> like that will scare users away when they given the alternative
> choice of using the small and easy-to-navigate old script in the
> same directory.
If you search the web for LSB conforming scripts, you'll find this is
one of the shorter ones.  I was intending to leave the old script
there, to satisfy those who want a short, simple script without LSB
compliance.
> So in summary my proposal would be to take the existing linux
> script, add the INFO header, and try to put all the other
> functionality that is missing into pg_ctl, making the init script a
> small wrapper around it.  That would actually benefit the mainstream
> init scripts as well.
I wouldn't object to adding the LSB comment header into the other
script, but it seems kinda pointless.  If you want that, I'd expect
you'd want compliant behavior, so that the OS can do the right thing
during startup and shutdown.  If we have consensus on making pg_ctl
behave very differently than it does now, to comply with the LSB
standard, we could have a much shorter script, for sure.  I didn't
expect we would get there, for backward compatibility reasons, if
nothing else.  I suppose I should have asked first.  So, who would
prefer that and who wouldn't?  If we do this, should it be based on
some switch, so that the old behavior is preserved unless
LSB-conformance is requested?
-Kevin