From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 27 Jun 2018 20:29:21 +0300 From: Konstantin Osipov Subject: Re: [PATCH 3/3] wal: create empty xlog on shutdown Message-ID: <20180627172921.GF28358@chai> References: <1499b12de12125f1258324b83e3fbb0e1d1d0587.1529075903.git.vdavydov.dev@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1499b12de12125f1258324b83e3fbb0e1d1d0587.1529075903.git.vdavydov.dev@gmail.com> To: Vladimir Davydov Cc: tarantool-patches@freelists.org List-ID: * Vladimir Davydov [18/06/15 23:28]: > recovery_finalize(struct recovery *r) > { > recovery_close_log(r); > - > - /* > - * Check if next xlog exists. If it's true this xlog is > - * corrupted and we should rename it (to avoid getting > - * problem on the next xlog write with the same name). > - * Possible reasons are: > - * - last xlog has corrupted rows > - * - last xlog has corrupted header > - * - last xlog has zero size > - */ > - char *name = xdir_format_filename(&r->wal_dir, > - vclock_sum(&r->vclock), > - NONE); > - if (access(name, F_OK) == 0) { > - say_info("rename corrupted xlog %s", name); > - char to[PATH_MAX]; > - snprintf(to, sizeof(to), "%s.corrupted", name); > - if (rename(name, to) != 0) { > - tnt_raise(SystemError, > - "%s: can't rename corrupted xlog", > - name); > - } > - } I agree this hunk should be moved to wal.c, but I don't understand why it has to be done in wal thread. Please make it obvious by leaving only the necessary parts in wal_init_f and adding a comment. > --- a/src/box/wal.c > +++ b/src/box/wal.c > @@ -310,6 +310,39 @@ wal_thread_start() > cpipe_set_max_input(&wal_thread.wal_pipe, IOV_MAX); > } > > +static int > +wal_init_f(struct cbus_call_msg *msg) Please add a formal comment for this function. > /** > * Initialize WAL writer. > * > @@ -332,6 +365,11 @@ wal_init(enum wal_mode wal_mode, const char *wal_dirname, > if (xdir_scan(&writer->wal_dir)) > return -1; > > + struct cbus_call_msg msg; > + if (cbus_call(&wal_thread.wal_pipe, &wal_thread.tx_pipe, &msg, > + wal_init_f, NULL, TIMEOUT_INFINITY) != 0) > + return -1; > + > + /* > + * Create a new empty WAL on shutdown so that we don't have > + * to rescan the last WAL to find the instance vclock. > + */ > + if (writer->wal_mode != WAL_NONE && > + xdir_create_xlog(&writer->wal_dir, &writer->current_wal, > + &writer->vclock) == 0) > + xlog_close(&writer->current_wal, false); In case there is an existing empty xlog file, this function fails to create a new one and returns an error which you ignore. Please handle this case nicely and avoid trying to create a new xlog file if it is not necessary. > -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov