From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id DDA4470202; Wed, 24 Feb 2021 00:49:27 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org DDA4470202 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1614116967; bh=pfL2nrrWUEXAYea641TIP70U2yfL56XrNFahkG4M1B8=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=jG2IGnbfxGoB37AmVZGLEgC0gg67llV3gDrs52DKuwOiOrUcQb1miakb7qPGanki9 b/G3sHJeABHnIdgnirUvFNWIf/dh3iKVQBphojRKjUW+AifsMDavYISFon4G20nbbg wSrTbi5LhDlMzMjCF1wK7mly+fyt/xDtzFyX6I1o= Received: from mail-lj1-f175.google.com (mail-lj1-f175.google.com [209.85.208.175]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 44F6070202 for ; Wed, 24 Feb 2021 00:49:25 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 44F6070202 Received: by mail-lj1-f175.google.com with SMTP id q23so4978533lji.8 for ; Tue, 23 Feb 2021 13:49:25 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=AwCadEDYR0cVntR4f4LMsqKLV0i5EhgaY6VrqsPX+w4=; b=aU+84bYxBcuiLAmygAjKxwj+mohNdqjbClBFNMnNswmx4GaAY7lSPN8R4BwRhbtm4X yst36CbycmhgelHKEAiwr+cOMVS++CAzwF7lB+l1OLbsp/A/xOECvF5GHa5C3ht2o7uY nJCfFpPcVVpqs1/oqnefTpoOvvCL0XGpoTrrtAxeWokGCtLvX5f+JQ6bXfzTKOLHU9V/ 8T5nXL1bKHuYVNR7Aa49T/Rxit+fXceQsYm0E5tdVDnWods4NOqkk9rUvOku2jQ6CRsU S0MLTV9ZEj+T/VDjENHpSw5/pDtm3VNITkB0bIi/bVPOXd0LCPOeR8aH6aShDkQpNYJ/ KuDQ== X-Gm-Message-State: AOAM530Yu20iq46svOc8n2b5j2bmkscQSNcnezRgy4ycgVmHU8guQ1zp a2t3P9x1UvD2rneOJjHbA2LECLFGPQA= X-Google-Smtp-Source: ABdhPJyQrgaytKuBp4QvUMZ4HO3hFA2oCt3yQq8QCgXSL3Hs1orDZbmeXNXdMYt74ccqAQq7E+0E7w== X-Received: by 2002:a2e:b0f9:: with SMTP id h25mr2875793ljl.232.1614116964147; Tue, 23 Feb 2021 13:49:24 -0800 (PST) Received: from grain.localdomain ([5.18.171.94]) by smtp.gmail.com with ESMTPSA id a30sm3165111ljq.96.2021.02.23.13.49.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Feb 2021 13:49:22 -0800 (PST) Received: by grain.localdomain (Postfix, from userid 1000) id E1A2F560158; Wed, 24 Feb 2021 00:49:21 +0300 (MSK) Date: Wed, 24 Feb 2021 00:49:21 +0300 To: Vladislav Shpilevoy Message-ID: References: <20210222182030.76232-1-gorcunov@gmail.com> <41811c65-448c-0de6-68cd-6a64779ca283@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <41811c65-448c-0de6-68cd-6a64779ca283@tarantool.org> User-Agent: Mutt/2.0.5 (2021-01-21) Subject: Re: [Tarantool-patches] [PATCH] say: fix format for fiber()->fid X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Cyrill Gorcunov via Tarantool-patches Reply-To: Cyrill Gorcunov Cc: tml Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On Tue, Feb 23, 2021 at 09:41:46PM +0100, Vladislav Shpilevoy wrote: > Hi! Please, add a changelog file. This is user-visible behaviour > so it must be reflected in the changes. > > On 22.02.2021 19:20, Cyrill Gorcunov wrote: > > The fiber's ID (fiber::fid) is unsigned integer so > > we should use a proper format specificator when printing > > it out, otherwise the logger show us weird strings like > > > > | main/-244760339/cartridge.failover.task I> Instance state changed > > > > Fixes #5846 > > > > Signed-off-by: Cyrill Gorcunov > > --- > > src/lib/core/say.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/src/lib/core/say.c b/src/lib/core/say.c > > index cbd10e107..6138752a7 100644 > > --- a/src/lib/core/say.c > > +++ b/src/lib/core/say.c > > @@ -792,7 +792,7 @@ say_format_plain_tail(char *buf, int len, int level, const char *filename, > > if (cord) { > > SNPRINT(total, snprintf, buf, len, " %s", cord->name); > > if (fiber() && fiber()->fid != FIBER_ID_SCHED) { > > - SNPRINT(total, snprintf, buf, len, "/%i/%s", > > + SNPRINT(total, snprintf, buf, len, "/%u/%s", > > fiber()->fid, fiber_name(fiber())); > > I remember we had some issues about %u not being compatible with uint32_t so > we did manual casts to 'unsigned'. Not sure though what was the commit/ticket, > or if it really happened. I suspect you rather mean uint64_t, which indeed better convert to excplicit "long long". Strictly speaking if we really need to cover compatibility we would _had_ to use PRIx macros which are so ugly :/ So no, uint32_t is known to be safe with %u format. Side note: I found that we still use %u for some of uint64_t, in particuar if (req->term < raft->volatile_term) { say_info("RAFT: the message is ignored due to outdated term - " "current term is %u", raft->volatile_term); return 0; } probably should use %llu and explicit "long long" for long standing servers where raft term would overflow 32 bit value. > > I am fine with not using the cast as long as all platforms and compilers > build flawlessly. Also it is ok for me if you will add an explicit cast in > the next version of the patch. Up to you. The same below. No, Vlad, I think we don't need explicit casts for 32bit values. I pushed an update to the branch gorcunov/gh-5846-fid-name and here is a new changelog. Does it look better? --- Subject: [PATCH] say: use unsigned format for fiber()->fid The fiber's ID (fiber::fid) is defined as unsigned integer so we should use a proper format specificator when printing it out. Currently logger prints such fibers as negative | main/-244760339/cartridge.failover.task I> Instance state changed why it should be | main/4271292179/cartridge.failover.task I> Instance state changed After the fix all fibers are logged with natural numbers (both in plain logger and json output formats) which might be considered as API change but should not cause any problems because if there were tools which scans for "zahlen" numbers they will scan for natural numbers without problems. After all there is no such things as fibers with negative numbers and when their fids are printed in Lua's "info" they are all natural (because they are converted into real numbers internally). Fixes #5846