From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Sat, 22 Sep 2018 19:21:43 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [tarantool-patches] Re: [PATCH v2] replication: fix assertion
 with duplicated connect to same master
Message-ID: <20180922162143.lmfywwcchzfw35ms@esperanza>
References: <20180911071105.88001-1-arkholga@tarantool.org>
 <20180917150512.fizr47fac4zks26l@esperanza>
 <85617ed6-0cc4-c354-dd62-7a84b9feae70@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <85617ed6-0cc4-c354-dd62-7a84b9feae70@tarantool.org>
To: Olga Arkhangelskaia <arkholga@tarantool.org>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

[Please don't drop the mailing list from Cc]

On Sat, Sep 22, 2018 at 10:04:57AM +0300, Olga Arkhangelskaia wrote:
> *
>=20
> Hi! Thanks for the review. I will fix most of the comments and observatio=
ns.
>=20
> However, I guess I need to know that we are on the same page.
>=20
> When we do box.cfg{replication=3D{44441, 44441}}
>=20
> we have two replicas, each one has its onw applier, etc. In
> replicaset_update()we are able to
>=20
> identify =A0that the replicas are same. At this point we raise exceptions.
> Problem occurs when we try to delete the second one. For proper deletion =
we
> need to stop applier, clear it and than delete replica. As I understand we
> need to:
>=20
> applier_stop(replica->applier);
>=20
> replica_clear_applier(replica);replica_delete(replica);

If you change the call sequence to

  replica_clear_applier(replica);
  replica_delete(replica);
  applier_stop(replica->applier);

you won't be facing this problem.

>=20
> The reason I added replica_on_applier_off(replica) is because when an
> applier enters stopped state, it state marks as APPLIER_OFF. Trigger on
> change states reacts on this change with
>=20
> replica_on_applier_state_f. That leads us to on_applier_sync. Instead we
> should react on APPLIER_DISCONNECTED. And the only way we react on this
> state =A0- is to try to load applier again.
>=20
> So replica_on_applier_off is used in case when we want to stop applier
> forever, before replica deletion. I think we do need this function. May be
> in some other form that I did.