From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-f195.google.com (mail-lj1-f195.google.com [209.85.208.195]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 5F1B8469719 for ; Thu, 19 Mar 2020 10:55:21 +0300 (MSK) Received: by mail-lj1-f195.google.com with SMTP id s13so1364586ljm.1 for ; Thu, 19 Mar 2020 00:55:21 -0700 (PDT) Date: Thu, 19 Mar 2020 10:55:19 +0300 From: Konstantin Osipov Message-ID: <20200319075519.GB3227@atlas> References: <49f33a8165d9de27ed5b852027b5093556fbb06a.1581500169.git.georgy@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <49f33a8165d9de27ed5b852027b5093556fbb06a.1581500169.git.georgy@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v4 01/11] recovery: do not call recovery_stop_local inside recovery_delete List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Georgy Kirichenko , sergepetrenko@tarantool.org Cc: tarantool-patches@dev.tarantool.org Hello, * Georgy Kirichenko [20/02/12 13:09]: > Recovery stop local raises an exception in case of an recovery error > so it is not safe to stop recovery inside recovery delete and guard > inside local_recovery. So call recovery_stop_local manually. I suggest you try/catch the exception in recovery_delete instead, or add a flag to recovery_stop_local to nothrow, or move diag_raise() out of recovery_stop_local() and make sure recovery_stop_local() returns 0, -1 It would be nice to explain in the changeset comment what is the exact event chain that leads to an exception from recovery_stop_local() - and even better - to add a test. > Part of #980 > --- > src/box/box.cc | 4 +++- > src/box/recovery.cc | 2 +- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/src/box/box.cc b/src/box/box.cc > index 1b2b27d61..68038df18 100644 > --- a/src/box/box.cc > +++ b/src/box/box.cc > @@ -2238,8 +2238,10 @@ local_recovery(const struct tt_uuid *instance_uuid, > recovery_follow_local(recovery, &wal_stream.base, "hot_standby", > cfg_getd("wal_dir_rescan_delay")); > while (true) { > - if (path_lock(cfg_gets("wal_dir"), &wal_dir_lock)) > + if (path_lock(cfg_gets("wal_dir"), &wal_dir_lock)) { > + recovery_stop_local(recovery); > diag_raise(); > + } > if (wal_dir_lock >= 0) > break; > fiber_sleep(0.1); > diff --git a/src/box/recovery.cc b/src/box/recovery.cc > index 64aa467b1..a1ac2d967 100644 > --- a/src/box/recovery.cc > +++ b/src/box/recovery.cc > @@ -216,7 +216,7 @@ gap_error: > void > recovery_delete(struct recovery *r) > { > - recovery_stop_local(r); > + assert(r->watcher == NULL); > > trigger_destroy(&r->on_close_log); > xdir_destroy(&r->wal_dir); > -- > 2.25.0 -- Konstantin Osipov, Moscow, Russia