From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id E8F3726333 for ; Thu, 28 Jun 2018 11:25:32 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id chig3s9buBVs for ; Thu, 28 Jun 2018 11:25:32 -0400 (EDT) Received: from smtp41.i.mail.ru (smtp41.i.mail.ru [94.100.177.101]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 9B6A0251F7 for ; Thu, 28 Jun 2018 11:25:32 -0400 (EDT) Received: by smtp41.i.mail.ru with esmtpa (envelope-from ) id 1fYYna-0008Nw-RC for tarantool-patches@freelists.org; Thu, 28 Jun 2018 18:25:31 +0300 Date: Thu, 28 Jun 2018 18:25:30 +0300 From: Konstantin Osipov Subject: [tarantool-patches] Re: [PATCH v3 2/2] replication: force gc to clean xdir on ENOSPC err Message-ID: <20180628152530.GA6042@chai> References: <2d8949f1e20f57f466d5ae56a131b86c5618a110.1530115423.git.k.belyavskiy@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2d8949f1e20f57f466d5ae56a131b86c5618a110.1530115423.git.k.belyavskiy@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org * Konstantin Belyavskiy [18/06/28 18:13]: > > +void > +gc_xdir_clean_notify() > +{ > + /* > + * Compare the current time with the time of the last run. > + * This is needed in case of multiple failures to prevent > + * from deleting all replicas. > + */ > + static int prev_time = 0; > + int cur_time = time(NULL); > + if (cur_time - prev_time < 1) Please use fiber time. > + /** > + * Mark consumer with least recent vclock as "dead" and > + * invoke garbage collection. If nothing to delete find > + * next alive consumer etc. Originally created for > + * cases with running out of disk space because of > + * disconnected replica. > + */ > + struct gc_consumer *leftmost = > + gc_tree_first(&gc.consumers); > + /* > + * Exit if no consumers left or if this consumer is > + * not associated with replica (backup for example). > + */ You should delete the consumer *only* if it's going to help with garbage. Running out of space should not lead to deletion of all replicas as consumers if they are not lagging behind. > + if (leftmost == NULL || leftmost->replica == NULL) > + return; > + int64_t signature = leftmost->signature; > + while (true) { > + gc_consumer_unregister(leftmost); > + leftmost = gc_tree_first(&gc.consumers); > + if (leftmost == NULL || leftmost->replica == NULL || > + leftmost->signature > signature) { > + gc_run(); > + return; > + } > + } > +} > + > const char *wal_mode_STRS[] = { "none", "write", "fsync", NULL }; > @@ -64,6 +65,8 @@ struct wal_thread { > * priority pipe and DOES NOT support yield. > */ > struct cpipe tx_prio_pipe; > + /** Return pipe from 'wal' to tx' */ > + struct cpipe tx_pipe; Why do you need a new pipe? > > +static void > +gc_status_update(struct cmsg *msg) > +{ > + gc_xdir_clean_notify(); > + delete(msg); delete()?! > { > @@ -655,6 +665,15 @@ done: > /* Until we can pass the error to tx, log it and clear. */ > error_log(error); > diag_clear(diag_get()); > + if (errno == ENOSPC) { > + struct cmsg *msg = > + (struct cmsg*)calloc(1, sizeof(struct cmsg)); Please check calloc() return value and do nothing in case it's NULL. > + static const struct cmsg_hop route[] = { > + {gc_status_update, NULL} > + }; > + cmsg_init(msg, route); > + cpipe_push(&wal_thread.tx_pipe, msg); > + } > > index 895d938d5..11f1b7fdc 100644 > +... > +-- add a little timeout so gc could finish job > +require('fiber').sleep(0.01) Please never sleep in a test case unconditionally. It doesn't work reliably, especially in parallel runs. The test case should test that replicas which are severely behind are abandoned, and replicas which are running well are not. -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov