From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp60.i.mail.ru (smtp60.i.mail.ru [217.69.128.40]) (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 16B574696C3 for ; Tue, 7 Apr 2020 13:46:10 +0300 (MSK) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.80.23.2.2\)) From: Serge Petrenko In-Reply-To: <20200404161524.7466-1-gorcunov@gmail.com> Date: Tue, 7 Apr 2020 13:46:08 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <5071F704-034B-4041-BB10-F048F61519CC@tarantool.org> References: <20200404161524.7466-1-gorcunov@gmail.com> Subject: Re: [Tarantool-patches] [PATCH v11 0/8] box/replication: prevent nil dereference on applier rollback List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov Cc: tml > 4 =D0=B0=D0=BF=D1=80. 2020 =D0=B3., =D0=B2 19:15, Cyrill Gorcunov = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0): >=20 > In the series a few fixups including simple code cleanup. >=20 > I've assigned a separate bug for myself for applier redesign > since I need more time to understand code better > https://github.com/tarantool/tarantool/issues/4853 >=20 > Issue https://github.com/tarantool/tarantool/issues/4730 > Branch gorcunov/gh-4730-diag-raise-master-11 >=20 > Cyrill Gorcunov (8): > box: fix bootstrap comment > box/alter: shrink txn_alter_trigger_new code > box/request: add missing OutOfMemory diag_set > box/applier: add missing diag_set on region_alloc failure > box/replication: merge replica_by_id into replicaset > applier: reduce applier_txn_rollback_cb code density > box/applier: prevent nil dereference on applier rollback > test: add replication/applier-rollback >=20 > src/box/alter.cc | 4 +- > src/box/applier.cc | 24 ++- > src/box/box.cc | 2 +- > src/box/replication.cc | 2 - > src/box/replication.h | 2 +- > src/box/request.c | 8 +- > src/box/txn.c | 13 ++ > src/lib/core/errinj.h | 1 + > test/box/errinj.result | 1 + > test/replication/applier-rollback-slave.lua | 16 ++ > test/replication/applier-rollback.result | 162 ++++++++++++++++++++ > test/replication/applier-rollback.test.lua | 81 ++++++++++ > test/replication/suite.ini | 2 +- > 13 files changed, 305 insertions(+), 13 deletions(-) > create mode 100644 test/replication/applier-rollback-slave.lua > create mode 100644 test/replication/applier-rollback.result > create mode 100644 test/replication/applier-rollback.test.lua >=20 > =E2=80=94=20 > 2.20.1 >=20 Hi! Thanks for the patchset! Commits 1,2, 5,6 LGTM except one comment: Please use =E2=80=98applier=E2=80=99 instead of =E2=80=98box/applier=E2=80= =99 prefixes in the commit titles. Similarly, =E2=80=98replication=E2=80=99 instead of = =E2=80=98box/replication=E2=80=99 and =E2=80=98alter=E2=80=99 instead of =E2=80=98box/alter=E2=80=99.=20 -- Serge Petrenko sergepetrenko@tarantool.org