From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp39.i.mail.ru (smtp39.i.mail.ru [94.100.177.99]) (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 F3D9746970E for ; Fri, 20 Dec 2019 15:17:32 +0300 (MSK) From: Serge Petrenko Message-Id: Content-Type: multipart/alternative; boundary="Apple-Mail=_7B04506F-05F0-4E69-A96A-554DDD9E8CF8" Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) Date: Fri, 20 Dec 2019 15:17:31 +0300 In-Reply-To: <42E14FD3-741F-45F3-B6CC-D4B655D473CC@tarantool.org> References: <1947329.0EtQL3PaDb@localhost> <42E14FD3-741F-45F3-B6CC-D4B655D473CC@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH 0/5] introduce anonymous replicas List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Georgy Kirichenko , Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org --Apple-Mail=_7B04506F-05F0-4E69-A96A-554DDD9E8CF8 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > 20 =D0=B4=D0=B5=D0=BA. 2019 =D0=B3., =D0=B2 15:07, Serge Petrenko = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0= =B0): >=20 >=20 >=20 >=20 >> 18 =D0=B4=D0=B5=D0=BA. 2019 =D0=B3., =D0=B2 10:49, Georgy Kirichenko = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0):= >>=20 >> Hi! >> Thank you for the patch set, good approach and overall it looks good = to be=20 >> merged. And there alos some points I would like to point. >> Please consider what should happen if any other instance tries to = connect to=20 >> an anonymous replica. I think we could enable such replication if a = connecting=20 >> replica is anonymous to, however, this should be an object of a = different issue=20 >> so we could disable such behavior right now. At least we should = handle=20 >> replica_id =3D 0 while replication. The case with replcia_id =3D 0 is handled in relay_send_row() Anonymous replica just doesn=E2=80=99t send the rows which have id =3D = 0. For now I just forbid replication from an anonymous instance. Other anonymous instances may fetch this instances snapshot,=20 and then they=E2=80=99ll get an error on subscribe: =C2=ABReplication does not support replicating from an anonymous = instance=C2=BB I=E2=80=99ve also opened a ticket to allow anon replication from an anon = instance https://github.com/tarantool/tarantool/issues/4696 -- Serge Petrenko sergepetrenko@tarantool.org >> The last point I would like to mention is that a test with = replication from an=20 >> anonymous replica could be included. >>=20 >>=20 > Hi! Thank you for review. I=E2=80=99ve added the test you requested = and sent it as a follow-up > to 5th patch. >=20 > Vlad, could you do the second review? > -- > Serge Petrenko > sergepetrenko@tarantool.org >=20 >=20 >> On Sunday, 15 December 2019 23:56:14 MSK sergepetrenko wrote: >>> Serge Petrenko (3): >>> box: update comment describing join protocol >>> replication: do not decode replicaset uuid when processing a = subscribe >>> applier: split join processing into two stages >>>=20 >>> sergepetrenko (2): >>> vclock: ignore 0th component in comparisons. >>> replication: introduce anonymous replica. >>>=20 >>> src/box/applier.cc | 119 ++++++++++++-- >>> src/box/applier.h | 4 + >>> src/box/box.cc | 278 = ++++++++++++++++++++++++++++++-- >>> src/box/box.h | 11 +- >>> src/box/iproto.cc | 16 +- >>> src/box/iproto_constants.h | 6 + >>> src/box/lua/cfg.cc | 14 +- >>> src/box/lua/info.c | 4 +- >>> src/box/lua/load_cfg.lua | 4 + >>> src/box/recovery.cc | 7 +- >>> src/box/relay.cc | 32 +++- >>> src/box/replication.cc | 41 ++++- >>> src/box/replication.h | 24 +++ >>> src/box/vclock.h | 7 + >>> src/box/wal.c | 4 + >>> src/box/xrow.c | 47 +++++- >>> src/box/xrow.h | 68 ++++++-- >>> test/app-tap/init_script.result | 49 +++--- >>> test/box/admin.result | 2 + >>> test/box/cfg.result | 4 + >>> test/replication/anon.lua | 13 ++ >>> test/replication/anon.result | 259 +++++++++++++++++++++++++++++ >>> test/replication/anon.test.lua | 89 ++++++++++ >>> test/replication/suite.cfg | 1 + >>> test/unit/vclock.cc | 8 +- >>> 25 files changed, 1022 insertions(+), 89 deletions(-) >>> create mode 100644 test/replication/anon.lua >>> create mode 100644 test/replication/anon.result >>> create mode 100644 test/replication/anon.test.lua >>=20 >=20 --Apple-Mail=_7B04506F-05F0-4E69-A96A-554DDD9E8CF8 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8





20 =D0=B4=D0=B5=D0=BA. 2019 =D0=B3., =D0=B2 15:07, Serge = Petrenko <sergepetrenko@tarantool.org> = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0):




18 =D0=B4=D0=B5=D0=BA. 2019 =D0=B3., =D0=B2 10:49, Georgy = Kirichenko <georgy@tarantool.org> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0= =D0=BB(=D0=B0):

Hi!
Thank you = for the patch set, good approach and overall it looks good to be
merged. And there alos some points I would like to point.
Please consider what should happen if any other instance = tries to connect to
an anonymous replica. I think we = could enable such replication if a connecting
replica is = anonymous to, however, this should be an object of a different issue
so we could disable such behavior right now. At least we = should handle
replica_id =3D 0 while replication.

The case with replcia_id =3D 0 is = handled in relay_send_row()
Anonymous replica just = doesn=E2=80=99t send the rows which have id =3D 0.

For now I just forbid = replication from an anonymous instance.
Other = anonymous instances may fetch this instances snapshot, 
and then they=E2=80=99ll get an error on subscribe:
=C2=ABReplication does not support replicating from an = anonymous instance=C2=BB

I=E2=80= =99ve also opened a ticket to allow anon replication from an anon = instance
https://github.com/tarantool/tarantool/issues/4696
--
Serge = Petrenko
sergepetrenko@tarantool.org

The last point I would = like to mention is that a test with replication from an
anonymous replica could be included.


Hi! Thank you for review. I=E2=80=99= ve added the test you requested and sent it as a follow-up
to 5th patch.

Vlad, could you do = the second review?
--
Serge Petrenko
sergepetrenko@tarantool.org


On = Sunday, 15 December 2019 23:56:14 MSK sergepetrenko wrote:
Serge Petrenko (3):
box: update comment describing join protocol
= replication: do not decode replicaset uuid when processing a = subscribe
applier: split join processing into two = stages

sergepetrenko (2):
= vclock: ignore 0th component in comparisons.
replication: = introduce anonymous replica.

src/box/applier.cc =             &n= bsp;| 119 ++++++++++++--
src/box/applier.h =             &n= bsp; |   4 +
src/box/box.cc =             &n= bsp;    | 278 ++++++++++++++++++++++++++++++--
src/box/box.h =             &n= bsp;     |  11 +-
src/box/iproto.cc =             &n= bsp; |  16 +-
src/box/iproto_constants.h =      |   6 +
src/box/lua/cfg.cc =             &n= bsp;|  14 +-
src/box/lua/info.c =             &n= bsp;|   4 +-
src/box/lua/load_cfg.lua =        |   4 +
src/box/recovery.cc =             | =   7 +-
src/box/relay.cc =             &n= bsp;  |  32 +++-
src/box/replication.cc =          |  41 = ++++-
src/box/replication.h =           |  24 = +++
src/box/vclock.h =             &n= bsp;  |   7 +
src/box/wal.c =             &n= bsp;     |   4 +
src/box/xrow.c =             &n= bsp;    |  47 +++++-
src/box/xrow.h= =             &n= bsp;    |  68 ++++++--
test/app-tap/init_script.result |  49 +++---
test/box/admin.result =           | =   2 +
test/box/cfg.result =             | =   4 +
test/replication/anon.lua =       |  13 ++
test/replication/anon.result    | 259 = +++++++++++++++++++++++++++++
test/replication/anon.test.lua=  |  89 ++++++++++
test/replication/suite.cfg =      |   1 +
test/unit/vclock.cc =             | =   8 +-
25 files changed, 1022 insertions(+), 89 = deletions(-)
create mode 100644 = test/replication/anon.lua
create mode 100644 = test/replication/anon.result
create mode 100644 = test/replication/anon.test.lua



= --Apple-Mail=_7B04506F-05F0-4E69-A96A-554DDD9E8CF8--