From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp34.i.mail.ru (smtp34.i.mail.ru [94.100.177.94]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id B032D469710 for ; Sun, 3 May 2020 19:45:37 +0300 (MSK) References: <20200503104629.GD23133@atlas> From: Vladislav Shpilevoy Message-ID: <557cd907-10fb-a98b-d048-506a40b91357@tarantool.org> Date: Sun, 3 May 2020 18:45:35 +0200 MIME-Version: 1.0 In-Reply-To: <20200503104629.GD23133@atlas> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH 1/1] wal: simplify rollback List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Konstantin Osipov , tarantool-patches@dev.tarantool.org, gorcunov@gmail.com, sergepetrenko@tarantool.org On 03/05/2020 12:46, Konstantin Osipov wrote: > * Vladislav Shpilevoy [20/05/01 22:16]: > > This isn't a particularly bad patch. It changes one fairly trivial > implementation to another. You claim it simplifies the rollback, I > believe it makes it more convoluted with the rest of the code. > > In any case it's a matter of taste - so perhaps your taste is > better than mine. > > But the reason you had to do this patch *only* because you want to manage > rollback in tx - as you claim yourself here: I didn't say it. All I said is this commit simplifies code, IMO. >> Also there is now a single route for commit and rollback cbus >> messages, called tx_complete_batch(). This change will come in >> hand in scope of synchronous replication, when WAL write won't be >> enough for commit. And therefore 'commit' as a concept should be >> washed away from WAL's code gradually. Migrate to solely txn >> module. > > The reason it has to be managed in TX is that you don't want to > build synchronous replication on top of in-memory relay, which > would allow you to manage RAFT rollback in WAL. Well, I already implemented some kind of base for sync replication on a different branch. It collects quorum and all. Next things are to implement 'confirm' and 'rollback' messages for WAL, timeouts. And it does not depend on this commit, or on any WAL-internal things at all. Sync replication needs only WAL API and does not care about its implementation. I sent this commit, because I was reading Georgy's implementation, saw this commit, decided to finish it because it looked useful, and only then realized I don't need it. Still decided to send it on a review, because it makes the code better as I think. And removes 'commit' concept from WAL. > When I called you out on the fact that you can't simply build your > implementation on top of existing code without an in-memory relay, > and it will require a bunch of hacks you got mad at me. I didn't get mad at you. You just started offending personally me out of nothing, and I stopped the talk, that is all story. Still sync replication *can be* built without in-memory relay; still not necessarily should be trigger-based (even though it is completely trigger-based in Georgy's implementation, regardless of in-memory WAL). And still you can't explain why in-memory WAL is needed except for probably better perf (and you can't prove it, because there is no a single bench). I propose you to finally come up with some proofs that in-memory relay is needed for anything, and is not just a userspace page-cache optimization. So far you couldn't provide anything. Luckily we have Cyrill G., who will bench this eventually, and see how much perf really goes to reading disk and decoding. > Now what? You will ask Nikita to review this and Kirill will > "LGTM" it and you will proceed with your own design - and even may This is actually funny - you don't like that we make 'our own design'? You would rather expect us make all design decisions only if they are authored and approved by you? Well, then you should just have stayed in the team. Or send your own patches, or at least your own RFCs. So far you only complain. You don't propose anything. You can't really expect us to wait for your approval on everything we do, including a simple refactoring patch. > release it as a "stable" version over some time. > > This "stable version may be even useful to some people. > > Will it be worth a dime on a worldwide scale? No. It seems to me you like thinking so. From how often you appeal to this. How the bad tarantool team wants only to find a way how to make 'their own very bad design decisions' not approved by you to offend personally you. You literally bring this up almost with every email you write here about any non-trivial patch. And you wouldn't get these toxic responses, if you wouldn't be toxic in the first place.