[patches] Re: [commits] [tarantool] 01/01: log: Fix log rotation is not async signal safe
Georgy Kirichenko
georgy at tarantool.org
Thu Feb 15 13:35:21 MSK 2018
I think SIGHUP signal handler should just send an event to ev_loop.
Corresponding event handler will do all job and can be safelly called from
other places (both calls in this case will be done synchronously without
sideeffects).
Current log_rotate is synchronous too, because a log_rotate stuck will freeze
the whole application even at signal handler. But it should be nice to do log
rotation through coio task.
On Tuesday, January 30, 2018 10:47:53 PM MSK Konstantin Osipov wrote:
> * IlyaMarkovMipt <markovilya197 at gmail.com> [18/01/24 13:34]:
> > commit a254d2b1cd8fb316ac3ad3ae36d59686d86cc8e8
> > Author: IlyaMarkovMipt <markovilya197 at gmail.com>
> > AuthorDate: Tue Jan 23 16:23:29 2018 +0300
> >
> > log: Fix log rotation is not async signal safe
> >
> > * Refactor signal handling with ev_signal.
> > * Add another signal watcher in main.
> >
> > We claim multiple loggers non-thread-safe,
> > so concurrency in logrotate signal handler
> > should not be considered.
>
> I'd be very cautious to make log rotation non-asynchronous.
> If we run out of disk space, and the logger goes to a file,
> write() can be completely stuck until there is available disk
> space. In this case the entire event loop will be stuck, and
> the system will sit in a dead lock.
>
> > Relates #3015
> >
> > ---
> >
> > src/lua/log.lua | 8 ++++++--
> > src/main.cc | 3 ++-
> > src/say.c | 7 ++++---
> > src/say.h | 5 ++++-
> > 4 files changed, 16 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/lua/log.lua b/src/lua/log.lua
> > index b90eb88..0ac0e8f 100644
> > --- a/src/lua/log.lua
> > +++ b/src/lua/log.lua
> > @@ -12,7 +12,11 @@ ffi.cdef[[
> >
> > extern sayfunc_t _say;
> >
> > - extern void say_logrotate(int);
> > + extern struct ev_loop;
> > + extern struct ev_signal;
> > +
> > + extern void
> > + say_logrotate(struct ev_loop *, struct ev_signal *, int);
> >
> > enum say_level {
> >
> > S_FATAL,
> >
> > @@ -104,7 +108,7 @@ local function say_closure(lvl)
> >
> > end
> >
> > local function log_rotate()
> >
> > - ffi.C.say_logrotate(0)
> > + ffi.C.say_logrotate(nil, nil, 0)
> >
> > end
> >
> > local function log_level(level)
> >
> > diff --git a/src/main.cc b/src/main.cc
> > index ec06103..1682bae 100644
> > --- a/src/main.cc
> > +++ b/src/main.cc
> > @@ -83,7 +83,7 @@ static char *pid_file = NULL;
> >
> > static char **main_argv;
> > static int main_argc;
> > /** Signals handled after start as part of the event loop. */
> >
> > -static ev_signal ev_sigs[5];
> > +static ev_signal ev_sigs[6];
> >
> > static const int ev_sig_count = sizeof(ev_sigs)/sizeof(*ev_sigs);
> >
> > static double start_time;
> >
> > @@ -335,6 +335,7 @@ signal_init(void)
> >
> > ev_signal_init(&ev_sigs[2], signal_cb, SIGTERM);
> > ev_signal_init(&ev_sigs[3], signal_cb, SIGHUP);
> > ev_signal_init(&ev_sigs[4], signal_sigwinch_cb, SIGWINCH);
> >
> > + ev_signal_init(&ev_sigs[5], say_logrotate, SIGHUP);
> >
> > for (int i = 0; i < ev_sig_count; i++)
> >
> > ev_signal_start(loop(), &ev_sigs[i]);
> >
> > diff --git a/src/say.c b/src/say.c
> > index 65c1872..394e6b3 100644
> > --- a/src/say.c
> > +++ b/src/say.c
> > @@ -269,9 +269,11 @@ log_rotate(const struct log *log)
> >
> > }
> >
> > void
> >
> > -say_logrotate(int signo)
> > +say_logrotate(struct ev_loop *loop, struct ev_signal *w, int revents)
> >
> > {
> >
> > - (void) signo;
> > + (void) loop;
> > + (void) w;
> > + (void) revents;
> >
> > int saved_errno = errno;
> > struct log *log;
> > rlist_foreach_entry(log, &log_rotate_list, in_log_list) {
> >
> > @@ -535,7 +537,6 @@ say_logger_init(const char *init_str, int level, int
> > nonblock,>
> > say_set_log_level(level);
> > log_background = background;
> > log_pid = log_default->pid;
> >
> > - signal(SIGHUP, say_logrotate);
> >
> > say_set_log_format(say_format_by_name(format));
> >
> > if (background) {
> >
> > diff --git a/src/say.h b/src/say.h
> > index 1b63229..46e6976 100644
> > --- a/src/say.h
> > +++ b/src/say.h
> > @@ -185,8 +185,11 @@ say_set_log_format(enum say_format format);
> >
> > enum say_format
> > say_format_by_name(const char *format);
> >
> > +struct ev_loop;
> > +struct ev_signal;
> > +
> >
> > void
> >
> > -say_logrotate(int /* signo */);
> > +say_logrotate(struct ev_loop *, struct ev_signal *, int /* revents */);
> >
> > /** Init default logger. */
> > void
More information about the Tarantool-patches
mailing list