Re: [Tarantool-patches] [PATCH v11 0/8] box/replication: prevent nil dereference on applier rollback

[Serge Petrenko <sergepetrenko@tarantool.org>]

> 4 апр. 2020 г., в 19:15, Cyrill Gorcunov <gorcunov@gmail.com> написал(а):
> 
> In the series a few fixups including simple code cleanup.
> 
> 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
> 
> Issue https://github.com/tarantool/tarantool/issues/4730
> Branch gorcunov/gh-4730-diag-raise-master-11
> 
> 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
> 
> 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
> 
> — 
> 2.20.1
> 

Hi! Thanks for the patchset!

Commits 1,2, 5,6 LGTM except one comment:
Please use ‘applier’ instead of ‘box/applier’
prefixes in the commit titles.
Similarly, ‘replication’ instead of ‘box/replication’ and
‘alter’ instead of ‘box/alter’. 
--
Serge Petrenko
sergepetrenko@tarantool.org