From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f51.google.com (mail-lf1-f51.google.com [209.85.167.51]) (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 DF4F44765E0 for ; Mon, 21 Dec 2020 23:02:40 +0300 (MSK) Received: by mail-lf1-f51.google.com with SMTP id h205so26588972lfd.5 for ; Mon, 21 Dec 2020 12:02:40 -0800 (PST) Date: Mon, 21 Dec 2020 23:02:26 +0300 From: Cyrill Gorcunov Message-ID: <20201221200226.GB81750@grain> References: <20201214113935.1040421-1-gorcunov@gmail.com> <20201214113935.1040421-3-gorcunov@gmail.com> <619b78fe-d559-7eb6-5e28-e239ed7f6e46@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <619b78fe-d559-7eb6-5e28-e239ed7f6e46@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v4 2/3] cfg: support symbolic evaluation of replication_synchro_quorum List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: Mons Anderson , tml On Mon, Dec 21, 2020 at 06:48:04PM +0100, Vladislav Shpilevoy wrote: > > + > > + if (lua_isnumber(tarantool_L, -1)) > > + quorum = (int)lua_tonumber(tarantool_L, -1); > > 1. There is a small issue: > > tarantool> box.cfg{replication_synchro_quorum='4294967297'} > 2020-12-21 18:33:16.015 [47366] main/103/interactive I> set 'replication_synchro_quorum' configuration option to "4294967297" Actually nope. When we pass this value it is treated as a plain number and cfg_geti trims it :( Here is a master branch output | tarantool> box.cfg{replication_synchro_quorum=4294967297} | ... | 2020-12-21 22:59:00.614 [176552] main/103/interactive I> set 'replication_synchro_quorum' configuration option to 4294967297 Note that on master branch I have to pass real numebr not string, but issue is the same... Need to think how to deal with it. > > + int value = MAX(1, replicaset.registered_count); > > + quorum = box_eval_replication_synchro_quorum(value); > > + if (quorum <= 0 || quorum >= VCLOCK_MAX) > > + panic("failed to eval replication_synchro_quorum"); > > 2. This check better be below. Because the numeric value also was > validated, right? True, will update.