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 7DBCB6EC55; Tue, 15 Jun 2021 23:46:16 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7DBCB6EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1623789976; bh=3uqJGRu4/GWtHQ50YvmssZp4zny/HLymryjCtaRgOXk=; 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=gMe5hcSI2D2eEcGZ2zJZTIa1i0n1fb1KcTwi0M7TIIqGRAho2VlI5QlLqjUqu7zjr pgqW/S6yF4e8qacuOlZf/kOZHmFgb+nltuiKLd3HbXuhHdDff86hlr9rDYVgdtI+22 ihduwjJJMVY7fy7jbhpMfPaU8ZF+AFCb41HYBMRI= Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com [209.85.167.52]) (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 BB0ED6EC55 for ; Tue, 15 Jun 2021 23:46:15 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org BB0ED6EC55 Received: by mail-lf1-f52.google.com with SMTP id f30so412674lfj.1 for ; Tue, 15 Jun 2021 13:46:15 -0700 (PDT) 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=C0j1nvJWi9RpDjT8zK52qWSBgxnNhpuxp+pEhkLpHMc=; b=YsgNuUCWeN/fM28/hbWSLo1Wa+HDpXVDlSWFBjo5yUb0bBrwvcWWYCQZ6dfJNeAD30 Sn+7R8sdnpLFG5m2j3Gm1oWS9EWI+fJmU1xOKOryVKD9lx+F1CwFVxX1qGKboEiI+uuB 9tqmEF5qQ2eU99mgm8u8mkQ6Ix5ramoBANYXKjT95rG9/zZ/qzmHmhmRylaspPtZLTHB jZ4WsIoPWMYJMs/hJ2+u1qi62xjnD+366rV+P+MfiDohKSoNA9/EDUFglsfVP5ZdiD/u fLoFZhHttumdSFNsHhWCQ4yjWKpZvYd1Znw312YSLZPlKxPoy2VYyFS/qYziXc6CsSlL ntxQ== X-Gm-Message-State: AOAM530ITFKl6MuHpb3z8ayWpGWtG/+kcexXqDU1jv6PLhCdMuusGEg4 sjEbi+nVfXbXqALaPgIWnhuCtCDxZn8= X-Google-Smtp-Source: ABdhPJzoiuy63nd1V9r2MODY8lanGUfOtn2BaB4zwzwQfalWPlE1yZMPoPb6OfTijB9rpRL86f25Gg== X-Received: by 2002:ac2:410e:: with SMTP id b14mr921003lfi.56.1623789974522; Tue, 15 Jun 2021 13:46:14 -0700 (PDT) Received: from grain.localdomain ([5.18.171.94]) by smtp.gmail.com with ESMTPSA id x1sm11364lfa.21.2021.06.15.13.46.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Jun 2021 13:46:13 -0700 (PDT) Received: by grain.localdomain (Postfix, from userid 1000) id A9DBC5A002A; Tue, 15 Jun 2021 23:46:12 +0300 (MSK) Date: Tue, 15 Jun 2021 23:46:12 +0300 To: Vladislav Shpilevoy Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/2.0.7 (2021-05-04) Subject: Re: [Tarantool-patches] [PATCH 04/13] wal: refactor wal_write_to_disk() 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: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On Fri, Jun 11, 2021 at 11:56:12PM +0200, Vladislav Shpilevoy wrote: > It didn't have a single fail path. That led to some amount of code > duplication, and it complicated future patches where the journal > entries are going to get a proper error reason instead of default > -1 without any details. > > The patch is a preparation for #6027 where it is wanted to have > more detailed errors on journal entry/transaction fail instead > of ER_WAL_IO for everything. Sometimes it can override a real > error like a cascade rollback, or a transaction conflict. > > Part of #6027 > --- > @@ -1038,7 +1036,10 @@ wal_write_to_disk(struct cmsg *msg) > { > struct wal_writer *writer = &wal_writer_singleton; > struct wal_msg *wal_msg = (struct wal_msg *) msg; > struct error *error; > + assert(!stailq_empty(&wal_msg->commit)); Hi Vlad, you know I don't understand why we need this assert... > /* > * Track all vclock changes made by this batch into > @@ -1058,23 +1059,17 @@ wal_write_to_disk(struct cmsg *msg) > > if (writer->is_in_rollback) { > /* We're rolling back a failed write. */ > - stailq_concat(&wal_msg->rollback, &wal_msg->commit); > - vclock_copy(&wal_msg->vclock, &writer->vclock); > - return; > + goto done; Jumps to "done" label change the code logic. Before the patch if we reached the write and say wal_opt_rotate failed we set up is_in_rollback sign and exit early, after the patch we start notifying watchers that there "write" happened which means relay code will be woken up while there no new data on disk level at all, which means watchers wanna be notified for no reason, no? Or I miss something obvious?