Re: [Tarantool-patches] [PATCH 1/1] wal: simplify rollback

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

On 03/05/2020 12:46, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [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.