Re: Linux LSB init script

From: Peter Eisentraut <peter_e(at)gmx(dot)net>
To: Kevin Grittner <Kevin(dot)Grittner(at)wicourts(dot)gov>
Cc: pgsql-hackers(at)postgresql(dot)org, Wolfgang Wilhelm <wolfgang20121964(at)yahoo(dot)de>
Subject: Re: Linux LSB init script
Date: 2009-09-17 05:30:15
Message-ID: 1253165415.18853.32.camel@vanquo.pezone.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On ons, 2009-09-16 at 12:05 -0500, Kevin Grittner wrote:
> > 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.

Yes, I didn't fully realize that the existing script didn't implement
all possible actions. But that can obviously be "backported" as well.

> > 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.

I think it's important to consider what the "conforming behavior" really
achieves in practice. The INFO section is essential nowadays for
correct functioning on a large portion of distributions. The exit codes
are relatively uninteresting but occasionally useful. The
init-functions don't make a difference at all, as far as I'm aware.

> > 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?

Well, you mentioned earlier that you were hesitant about backporting the
INFO header without adding all the other "conforming" stuff. From a
practical point of view, I think an INFO header is nowadays essential
independent of what the rest of the script does.

> > 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?

Well, in such cases it may be useful to add an option such as --oknodo
to select the idempotent behavior.

> > 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.

Right, but if you think that this behavior the right/better one (which
it arguably isn't), why shouldn't it be available from the mainline
tools? Obviously, most people have felt for the last N years that
pg_ctl provides adequate shutdown options. If not, then let's look
there. We shouldn't hardcode an alternative policy into a marginal init
script, which it will receive little testing and scrutiny.

> > 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.

Yeah, except that part of the spec is hilariously unrealistic. And
there is no evidence to suggest that use of these functions will cause
messages to be formatted like other services on the machine.
Debian/Ubuntu for example have given up on those functions because they
don't anything useful and provide their own set of functions that you
ought to use to format messages like on those systems.

> > 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 or service 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, exit should be 3
> for those.

I would pretty much remove all of that completely and replace it with a
link to the spec.

> 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?

How about making a list of current behavior of pg_ctl vs. required
behavior under LSB, and then we can judge how much of that could become
a standard behavior and how much would have to be hidden behind an
option.

I think figuring this out could be a key improvement; all the stuff
above is just cosmetic after all.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2009-09-17 06:54:03 Re: Hot Standby 0.2.1
Previous Message Heikki Linnakangas 2009-09-17 05:12:09 Re: Feedback on getting rid of VACUUM FULL