From c4c59a094d2e355d5bbe90c6dc7e92473a35380d Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Sun, 22 Apr 2012 14:35:39 -0700 Subject: [PATCH] log: do not set on_exit() callback for libraries Set this up in either global_init() or common_init_finish(), both opportune times that occur after config parsing has happened and the user has the option to modify this behavior. The exception would be libraries like librados, which can't use rados_conf_* to enable this. Arguably flush functionality should be exposed through the librados API directly, instead of futzing with on_exit(). Signed-off-by: Sage Weil --- src/common/common_init.cc | 4 ++++ src/common/config_opts.h | 12 ++++++++---- src/global/global_init.cc | 3 +++ src/log/Log.cc | 24 ++++++++++++++++-------- src/log/Log.h | 2 ++ 5 files changed, 33 insertions(+), 12 deletions(-) diff --git a/src/common/common_init.cc b/src/common/common_init.cc index 6562ae910ea..e4617ea9af7 100644 --- a/src/common/common_init.cc +++ b/src/common/common_init.cc @@ -58,6 +58,7 @@ CephContext *common_preinit(const CephInitParameters &iparams, case CODE_ENVIRONMENT_LIBRARY: conf->set_val_or_die("log_to_stderr", "false"); conf->set_val_or_die("err_to_stderr", "false"); + conf->set_val_or_die("log_flush_on_exit", "false"); break; default: @@ -103,6 +104,9 @@ void common_init_finish(CephContext *cct) ceph::crypto::init(); cct->start_service_thread(); + if (cct->_conf->log_flush_on_exit) + cct->_log->set_flush_on_exit(); + // Trigger callbacks on any config observers that were waiting for // it to become safe to start threads. cct->_conf->set_val("internal_safe_to_start_threads", "true"); diff --git a/src/common/config_opts.h b/src/common/config_opts.h index 75d7c16d7b3..7643cda9839 100644 --- a/src/common/config_opts.h +++ b/src/common/config_opts.h @@ -22,9 +22,14 @@ OPTION(cluster_network, OPT_STR, "") OPTION(num_client, OPT_INT, 1) OPTION(monmap, OPT_STR, "") OPTION(mon_host, OPT_STR, "") -OPTION(daemonize, OPT_BOOL, false) OPTION(lockdep, OPT_BOOL, false) OPTION(admin_socket, OPT_STR, "/var/run/ceph/$cluster.name.asok") + +OPTION(daemonize, OPT_BOOL, false) +OPTION(pid_file, OPT_STR, "") +OPTION(chdir, OPT_STR, "/") +OPTION(max_open_files, OPT_LONGLONG, 0) + OPTION(log_file, OPT_STR, "/var/log/ceph/$cluster.$name.log") OPTION(log_max_new, OPT_INT, 1000) OPTION(log_max_recent, OPT_INT, 1000000) @@ -32,11 +37,10 @@ OPTION(log_to_stderr, OPT_BOOL, true) OPTION(err_to_stderr, OPT_BOOL, true) OPTION(log_to_syslog, OPT_BOOL, false) OPTION(err_to_syslog, OPT_BOOL, false) +OPTION(log_flush_on_exit, OPT_BOOL, true) + OPTION(clog_to_monitors, OPT_BOOL, true) OPTION(clog_to_syslog, OPT_BOOL, false) -OPTION(pid_file, OPT_STR, "") -OPTION(chdir, OPT_STR, "/") -OPTION(max_open_files, OPT_LONGLONG, 0) DEFAULT_SUBSYS(0, 5) SUBSYS(lockdep, 0, 5) diff --git a/src/global/global_init.cc b/src/global/global_init.cc index 44db4c14810..90c1edc0451 100644 --- a/src/global/global_init.cc +++ b/src/global/global_init.cc @@ -107,6 +107,9 @@ void global_init(std::vector < const char * > *alt_def_args, std::vector < const block_signals(siglist, NULL); install_standard_sighandlers(); + if (g_conf->log_flush_on_exit) + g_ceph_context->_log->set_flush_on_exit(); + if (g_lockdep) { dout(1) << "lockdep is enabled" << dendl; lockdep_register_ceph_context(cct); diff --git a/src/log/Log.cc b/src/log/Log.cc index 9cb90c15422..f029390f5a5 100644 --- a/src/log/Log.cc +++ b/src/log/Log.cc @@ -30,7 +30,7 @@ static void log_on_exit(int r, void *p) } Log::Log(SubsystemMap *s) - : m_indirect_this(new (Log*)(this)), // we will deliberately leak this + : m_indirect_this(NULL), m_subs(s), m_new(), m_recent(), m_fd(-1), @@ -42,12 +42,6 @@ Log::Log(SubsystemMap *s) { int ret; - // Make sure we flush on shutdown. We do this by deliberately - // leaking an indirect pointer to ourselves (on_exit() can't - // unregister a callback). This is not racy only becuase we - // assume that exit() won't race with ~Log(). - on_exit(log_on_exit, m_indirect_this); - ret = pthread_spin_init(&m_lock, PTHREAD_PROCESS_SHARED); assert(ret == 0); @@ -68,7 +62,9 @@ Log::Log(SubsystemMap *s) Log::~Log() { - *m_indirect_this = NULL; + if (m_indirect_this) { + *m_indirect_this = NULL; + } assert(!is_started()); if (m_fd >= 0) @@ -83,6 +79,18 @@ Log::~Log() /// +void Log::set_flush_on_exit() +{ + // Make sure we flush on shutdown. We do this by deliberately + // leaking an indirect pointer to ourselves (on_exit() can't + // unregister a callback). This is not racy only becuase we + // assume that exit() won't race with ~Log(). + if (m_indirect_this == NULL) { + m_indirect_this = new (Log*)(this); + on_exit(log_on_exit, m_indirect_this); + } +} + void Log::set_max_new(int n) { m_max_new = n; diff --git a/src/log/Log.h b/src/log/Log.h index 372cc789ba4..ab83a7c86e4 100644 --- a/src/log/Log.h +++ b/src/log/Log.h @@ -49,6 +49,8 @@ public: Log(SubsystemMap *s); virtual ~Log(); + void set_flush_on_exit(); + void set_max_new(int n); void set_max_recent(int n); void set_log_file(std::string fn); -- 2.47.3