[PATCH] daemons: write pid file even when told not to daemonize

Hi Alexandre,

With this patch ceph-osd -f will try to create the default pid file : this is a non backward compatible change. Maybe there is a way for systemd to capture the pid of the process and store it instead of requiring the deamon to create the pid file ?

Cheers

Post by Alexandre Oliva
systemd wants to run daemons in foreground, but daemons wouldn't write
out the pid file with -f. Fixed.
---
src/ceph_mon.cc | 3 +--
src/common/config.cc | 2 --
src/global/global_init.cc | 10 +++++++++-
3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/src/ceph_mon.cc b/src/ceph_mon.cc
index 4e84b4d..14dd6da 100644
--- a/src/ceph_mon.cc
+++ b/src/ceph_mon.cc
@@ -406,8 +406,7 @@ int main(int argc, const char **argv)
// screwing us over
Preforker prefork;
if (!(flags & CINIT_FLAG_NO_DAEMON_ACTIONS)) {
- if (g_conf->daemonize) {
- global_init_prefork(g_ceph_context, 0);
+ if (global_init_prefork(g_ceph_context, 0) >= 0) {
prefork.prefork();
if (prefork.is_parent()) {
return prefork.parent_wait();
diff --git a/src/common/config.cc b/src/common/config.cc
index 0ee7f58..4e3b6fe 100644
--- a/src/common/config.cc
+++ b/src/common/config.cc
@@ -389,12 +389,10 @@ int md_config_t::parse_argv(std::vector<const char*>& args)
}
else if (ceph_argparse_flag(args, i, "--foreground", "-f", (char*)NULL)) {
set_val_or_die("daemonize", "false");
- set_val_or_die("pid_file", "");
}
else if (ceph_argparse_flag(args, i, "-d", (char*)NULL)) {
set_val_or_die("daemonize", "false");
set_val_or_die("log_file", "");
- set_val_or_die("pid_file", "");
set_val_or_die("log_to_stderr", "true");
set_val_or_die("err_to_stderr", "true");
set_val_or_die("log_to_syslog", "false");
diff --git a/src/global/global_init.cc b/src/global/global_init.cc
index 7b20343..f03677c 100644
--- a/src/global/global_init.cc
+++ b/src/global/global_init.cc
@@ -166,8 +166,16 @@ int global_init_prefork(CephContext *cct, int flags)
if (g_code_env != CODE_ENVIRONMENT_DAEMON)
return -1;
const md_config_t *conf = cct->_conf;
- if (!conf->daemonize)
+ if (!conf->daemonize) {
+ if (atexit(pidfile_remove_void)) {
+ derr << "global_init_daemonize: failed to set pidfile_remove function "
+ << "to run at exit." << dendl;
+ }
+
+ pidfile_write(g_conf);
+
return -1;
+ }
// stop log thread
g_ceph_context->_log->flush();

--
Loïc Dachary, Artisan Logiciel Libre

Sage Weil

2014-07-31 04:30:09 UTC

Post by Loic Dachary
Hi Alexandre,
this is a non backward compatible change. Maybe there is a way for
systemd to capture the pid of the process and store it instead of
requiring the deamon to create the pid file ?

Do we need the pid file at all when we aren't using sysinit?

sage

Post by Loic Dachary
Cheers

--
Lo?c Dachary, Artisan Logiciel Libre

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Alexandre Oliva

2014-08-28 07:35:10 UTC

Do we need the pid file at all when we aren't using sysinit?

My own monitoring scripts use it, and ceph stop use it as well. I was
surprised we were not creating them in spite of an explicit command line
option to do so.

Loic Dachary

2014-08-28 08:09:28 UTC

Hi Alexandre,

Changing this behavior is not backward compatible but it is indeed more intuitive. Has it been a significant inconvenience so far ?

Cheers

Do we need the pid file at all when we aren't using sysinit?

My own monitoring scripts use it, and ceph stop use it as well. I was
surprised we were not creating them in spite of an explicit command line
option to do so.

--
Loïc Dachary, Artisan Logiciel Libre

Alexandre Oliva

2014-08-29 06:41:18 UTC

Post by Loic Dachary
Changing this behavior is not backward compatible but it is indeed more intuitive. Has it been a significant inconvenience so far ?

Before I wrote the patch, it was very inconvenient: in order to stop
ceph services, I had to dig up the PIDs from ps output and then kill the
processes manually, and I had to run ceph without my monitor that
automatically restarted services that died (checking that the pid file
was absent, or that the PID it named was not a running process).

After I applied the patch in my local build, I could just forget about
the earlier problems, and none surfaced because of the creation of the
PID file.

Is this what you were asking?

Post by Loic Dachary

Do we need the pid file at all when we aren't using sysinit?

My own monitoring scripts use it, and ceph stop use it as well. I was
surprised we were not creating them in spite of an explicit command line
option to do so.

Loic Dachary

2014-08-29 08:40:43 UTC

Post by Loic Dachary
Changing this behavior is not backward compatible but it is indeed more intuitive. Has it been a significant inconvenience so far ?

Before I wrote the patch, it was very inconvenient: in order to stop
ceph services, I had to dig up the PIDs from ps output and then kill the
processes manually, and I had to run ceph without my monitor that
automatically restarted services that died (checking that the pid file
was absent, or that the PID it named was not a running process).
After I applied the patch in my local build, I could just forget about
the earlier problems, and none surfaced because of the creation of the
PID file.
Is this what you were asking?

Hi Alexandre,

After a quick glance at the systemd man page http://www.freedesktop.org/software/systemd/man/systemd.service.html#Options it seems possible to do something like

https://github.com/dachary/ceph/compare/wip-systemd?expand=1

Alternatively, shouldn't systemctl kill ceph-***@10.service be the canonical way of killing a process spawned by systemd ?

If both are impractical or frowned upon for some reason, it looks like your patch is the only way to go, indeed.

Cheers

Post by Loic Dachary

Do we need the pid file at all when we aren't using sysinit?

My own monitoring scripts use it, and ceph stop use it as well. I was
surprised we were not creating them in spite of an explicit command line
option to do so.

--
Loïc Dachary, Artisan Logiciel Libre

Alexandre Oliva

2014-09-04 19:48:04 UTC

After a quick glance at the systemd man page http://www.freedesktop.o=

rg/software/systemd/man/systemd.service.html#Options it seems possible =
to do something like

https://github.com/dachary/ceph/compare/wip-systemd?expand=3D1

Yeah, it sure is possible in systemd to start and track daemons that
don't have a =E2=80=9Crun in foreground=E2=80=9D option, but it's not a=
s efficient.
nonical way of killing a process spawned by systemd ?

If you know what number to use after @, I guess it would. I don't thin=
k
we're talking about the same version of ceph, when it comes to systemd
config files. Your patch doesn't modify any file present on my system
(still running 0.80.5)

If both are impractical or frowned upon for some reason, it looks lik=

your patch is the only way to go, indeed.

It's certainly not the only way to go, but silently dropping a command
line option sounds like a bug to me. Of course the patch I proposed
isn't the only way to go about that...

--=20
Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
=46ree Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Sage Weil

2014-09-09 22:21:16 UTC

I'm inlined to just apply this original patch. Is there any reason *not*
to create a pid file when -f is used? It's a change from previous
behavior, but is there any possible harm that can come of it?

sage

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Loic Dachary

2014-09-09 22:48:40 UTC

The only reason I can think of is that it requires write permission to a directory that was previously not used. I think it will break a few unit tests but nothing we can't fix.

Post by Sage Weil
I'm inlined to just apply this original patch. Is there any reason *not*
to create a pid file when -f is used? It's a change from previous
behavior, but is there any possible harm that can come of it?
sage

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Loïc Dachary, Artisan Logiciel Libre

Sage Weil

2014-09-09 22:54:11 UTC

Post by Loic Dachary
The only reason I can think of is that it requires write permission to a
directory that was previously not used. I think it will break a few unit
tests but nothing we can't fix.

Let's go for it then and clean up whatever breaks. We may want to change
the default value for pid_file for some contexts, but I agree with
Alexandre that that will be a better situation that the funny -f/-d
side-effect we have now!

sage

Post by Loic Dachary

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Lo?c Dachary, Artisan Logiciel Libre

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Loic Dachary

2014-09-12 10:22:32 UTC