From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-f196.google.com (mail-lj1-f196.google.com [209.85.208.196]) (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 AA33A469719 for ; Fri, 14 Feb 2020 13:52:35 +0300 (MSK) Received: by mail-lj1-f196.google.com with SMTP id o15so10210845ljg.6 for ; Fri, 14 Feb 2020 02:52:35 -0800 (PST) Date: Fri, 14 Feb 2020 13:52:33 +0300 From: Konstantin Osipov Message-ID: <20200214105233.GA2696@atlas> References: <20200214072526.GC15237@atlas> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH v2 4/4] replication: do not promote local_vclock_at_subscribe unnecessarily List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Serge Petrenko Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org * Serge Petrenko [20/02/14 13:47]: > The fix allows to stop relaying rows that have just came from the replica back to it. > It is not necessary, since we’ve fixed applier in a different way, but I think there’s no > need to resend the replica’s rows back and forth. It just looks more correct, or consistent, > if you wish. If master responds that its vclock is such and such, it should use the same > vclock to judge whether to send the replica its own rows, or not, in my opinion. > Before the patch master judges by replicaset vclock, which may get updated while master > responds to subscribe (coio_write_xrow yields). These rows are few. The check is there in all cases. There is no such thing as partially rotten egg. The old code was correct in that regard. The new code can't be "more correct", it's either also correct or not. If you want to clarify the old code, you could add a comment. > Yes, master sends it to replica, and later master uses its own replicaset vclock > (previously) or the same vclock it sent to replica (after my patch) to filter replica’s > rows to send back to it (see the piece of code you’ve shown me yesterday). No, it uses a different vclock for filtering - replica vclock. > Imagine a situation: you have master-master configuration with instances 1 and 2 > 2 is subscribed to 1, and 1 resubscribes to 2 (maybe 2 just restarted and was the first one > to successfully subscribe). > 2 yields on writing subscribe responce. In the meantime 1 writes something new to WAL > and relays it to 2. 2 writes it to WAL and increments its replicaset vclock. Later it’ll resend > these rows back to 1, because `relay->local_vclock_at_subscribe` holds an updated vclock > value. > Hope I made it more clear this time. This is what you should fix by setting replica_vclock[self] = infinity at subscribe. Do you see the difference between a single assignment instruction and a condition evaluated on every row? -- Konstantin Osipov, Moscow, Russia