INDY-1404: Do not do view change until catchup is finished #757

ashcherbakov · 2018-06-19T15:05:43Z

do not process any View Change related messages (including ViewChangeDone from Current State for primary propagation) until the current catchup is finished;
this fixes initialization of a node where we mixed together initial catchup and primary propagation (initial view change);
also fixed recovering when primary is disconnected and (n-f)th node joins the pool

Signed-off-by: ashcherbakov alexander.sherbakov@dsr-company.com

- do not process any View Change related messages (including ViewChangeDone from Current State for primary propagation) until the current catchup is finished; - this fixes initialization of a node where we mixed together initial catchup and primary propagation (initial view change); - also fixed recovering when primary is disconnected and (n-f)th node joins the pool Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

andkononykhin · 2018-06-20T12:40:15Z

test this please

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

andkononykhin · 2018-06-20T14:35:02Z

test this please

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

…up-cancellation

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

Signed-off-by: ArtObr <artemobruchnikov@gmail.com>

fix for failing test

ArtObr · 2018-06-22T07:34:33Z

test this please

…rledger/indy-plenum into catchup-cancellation

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

…rledger/indy-plenum into catchup-cancellation

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

skhoroshavin · 2018-06-28T13:11:29Z

plenum/common/messages/node_messages.py

@@ -404,3 +404,9 @@ def _validate_message(self, dct):
        self._raise_invalid_fields(
            f.MSG.nm, msg,
            "The message type must be {} ".format(expected_type_cls.typename))
+
+
+class FutureViewChangeDone:


It would be great if purpose of this message was documented, as with all other messages in this file

skhoroshavin · 2018-06-28T14:07:40Z

plenum/server/node.py

@@ -1905,6 +1909,9 @@ def is_catchup_needed(self) -> bool:
        Check if all requests ordered till last prepared certificate
        Check if last catchup resulted in no txns
        """
+        if not self.view_change_in_progress:


Looks a bit counterintuitive, it would be nice to have a comment explaining why no catchup is needed if view change is not in progress

skhoroshavin · 2018-06-28T14:21:35Z

plenum/server/view_change/view_changer.py

+        # This is especially critical for Propagate Primary mode (on receiving CURRENT_STATE by a new node).
+        if self.view_no in self._next_view_indications:
+            for frm, vcd in self._next_view_indications[self.view_no].items():
+                self._on_verified_view_change_done_msg(vcd, frm)


Probably it's better to call process_vchd_msg here, which will call _on_verified_view_change_done_msg only when it's truly verified?

It will not work. Added a comment with a description.

skhoroshavin · 2018-06-28T14:26:12Z

plenum/server/view_change/view_changer.py

+        # remove all for previous views
+        for view_no in tuple(self._next_view_indications.keys()):
+            if view_no <= self.view_no:
+                self._next_view_indications.pop(view_no)


del self._next_view_indications[view_no] could be more appropriate, since pop returns removed item which we don't actually need

skhoroshavin · 2018-06-28T14:49:51Z

plenum/test/view_change/test_no_future_view_change_while_catchup.py

+        ensure_view_change(looper, other_nodes)
+        checkProtocolInstanceSetup(looper=looper, nodes=other_nodes, numInstances=2)
+        ensure_all_nodes_have_same_data(looper, nodes=other_nodes)
+        looper.runFor(expectedPoolViewChangeStartedTimeout(len(txnPoolNodeSet)))


This looks like a probably source of flakiness. Can we come up with some predicate which can indicate that view change won't happen for sure?

skhoroshavin · 2018-06-28T14:50:04Z

plenum/test/view_change/test_no_view_change_while_catchup.py

+    old_meths = do_view_change(txnPoolNodeSet)
+    for node in txnPoolNodeSet:
+        node.view_changer.sendInstanceChange(old_view_no + 1)
+    looper.runFor(expectedPoolViewChangeStartedTimeout(len(txnPoolNodeSet)))


This looks like a probably source of flakiness. Can we come up with some predicate which can indicate that view change won't happen for sure?

skhoroshavin · 2018-06-28T14:52:33Z

plenum/test/view_change/test_no_future_view_change_while_catchup.py

+from plenum.test.waits import expectedPoolViewChangeStartedTimeout
+
+
+@pytest.fixture(params=['starting', 'discovering', 'discovered', 'syncing'])


Do we really need strings here and lot's of if/else? Would the following do the job?

@pytest.fixture(params=[Mode.starting, Mode.discovering...]) def mode(request) return request.param

skhoroshavin · 2018-06-28T14:52:45Z

plenum/test/view_change/test_no_view_change_while_catchup.py

+from plenum.test.waits import expectedPoolViewChangeStartedTimeout
+
+
+@pytest.fixture(params=['starting', 'discovering', 'discovered', 'syncing'])


Do we really need strings here and lot's of if/else? Would the following do the job?

@pytest.fixture(params=[Mode.starting, Mode.discovering...]) def mode(request) return request.param

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

ashcherbakov · 2018-06-29T06:37:33Z

test this please

ashcherbakov · 2018-06-29T07:29:34Z

test this please

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

ashcherbakov · 2018-06-29T09:20:30Z

test this please

ashcherbakov · 2018-06-29T11:30:15Z

test this please

ashcherbakov · 2018-06-29T12:31:08Z

test this please

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

ashcherbakov · 2018-07-02T05:58:24Z

test this please

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

…hup-cancellation Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com> # Conflicts: # plenum/test/recorder/test_replay_with_view_change.py

ashcherbakov changed the title ~~Do not do view change until catchup is finished~~ [WIP] INDY-1404: Do not do view change until catchup is finished Jun 19, 2018

ashcherbakov added 2 commits June 19, 2018 18:46

fix flake8

5a3cf66

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

fix test

191b65f

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

andkononykhin added 2 commits June 20, 2018 16:06

Merge remote-tracking branch 'public/master' into catchup-cancellation

9e6c4e2

fixes missed import

39ce621

Signed-off-by: Andrey Kononykhin <andkononykhin@gmail.com>

ashcherbakov and others added 5 commits June 21, 2018 11:54

improve tests

05b2808

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

Merge remote-tracking branch 'origin/catchup-cancellation' into catch…

131a45f

…up-cancellation

do not start view change until the status is synced

e2bc0ff

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

fix for failing test

e07416f

Signed-off-by: ArtObr <artemobruchnikov@gmail.com>

Merge pull request #765 from ArtObr/catchup-cancellation

580570a

fix for failing test

ashcherbakov added 8 commits June 25, 2018 12:11

Merge branches 'catchup-cancellation' and 'master' of github.com:hype…

3551b1c

…rledger/indy-plenum into catchup-cancellation

fix restart tests

e90ebc6

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

fix tests in plugin

48a2154

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

fix initial propose view change on primary disconnection

c9997d4

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

fix flake8

611faba

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

fix tests

dbdc442

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

fix tests

63ee812

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

Merge branches 'catchup-cancellation' and 'master' of github.com:hype…

442c3a1

…rledger/indy-plenum into catchup-cancellation

ashcherbakov changed the title ~~[WIP] INDY-1404: Do not do view change until catchup is finished~~ INDY-1404: Do not do view change until catchup is finished Jun 28, 2018

rename tests for better description

3af7547

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

skhoroshavin reviewed Jun 28, 2018

View reviewed changes

ashcherbakov added 2 commits June 28, 2018 18:59

improve tests, add comments

5fccd2b

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

improve tests

2705cb2

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

fix stasher

f6e694b

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

ashcherbakov added 2 commits June 29, 2018 17:25

skip too long tests and avoid too big folders with tests

393da7c

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

fix tests

d690cd7

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

fix tests

54b8464

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

skhoroshavin previously approved these changes Jul 2, 2018

View reviewed changes

ashcherbakov added 2 commits July 2, 2018 12:10

make the test more deterministic

be2b994

Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com>

Merge branch 'master' of github.com:hyperledger/indy-plenum into catc…

9b73345

…hup-cancellation Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-company.com> # Conflicts: # plenum/test/recorder/test_replay_with_view_change.py

ashcherbakov dismissed skhoroshavin’s stale review via 9b73345 July 2, 2018 09:12

skhoroshavin approved these changes Jul 2, 2018

View reviewed changes

andkononykhin approved these changes Jul 2, 2018

View reviewed changes

andkononykhin merged commit deb82df into master Jul 2, 2018

ashcherbakov deleted the catchup-cancellation branch July 3, 2018 08:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

INDY-1404: Do not do view change until catchup is finished #757

INDY-1404: Do not do view change until catchup is finished #757

ashcherbakov commented Jun 19, 2018

andkononykhin commented Jun 20, 2018

andkononykhin commented Jun 20, 2018

ArtObr commented Jun 22, 2018

skhoroshavin Jun 28, 2018

ashcherbakov Jun 28, 2018

skhoroshavin Jun 28, 2018

ashcherbakov Jun 28, 2018

skhoroshavin Jun 28, 2018

ashcherbakov Jun 28, 2018

skhoroshavin Jun 28, 2018

ashcherbakov Jun 28, 2018

skhoroshavin Jun 28, 2018

skhoroshavin Jun 28, 2018

ashcherbakov Jun 28, 2018

skhoroshavin Jun 28, 2018

skhoroshavin Jun 28, 2018

ashcherbakov Jun 28, 2018

ashcherbakov commented Jun 29, 2018

ashcherbakov commented Jun 29, 2018

ashcherbakov commented Jun 29, 2018

ashcherbakov commented Jun 29, 2018

ashcherbakov commented Jun 29, 2018

ashcherbakov commented Jul 2, 2018

		from plenum.test.waits import expectedPoolViewChangeStartedTimeout


		@pytest.fixture(params=['starting', 'discovering', 'discovered', 'syncing'])

INDY-1404: Do not do view change until catchup is finished #757

INDY-1404: Do not do view change until catchup is finished #757

Conversation

ashcherbakov commented Jun 19, 2018

andkononykhin commented Jun 20, 2018

andkononykhin commented Jun 20, 2018

ArtObr commented Jun 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashcherbakov commented Jun 29, 2018

ashcherbakov commented Jun 29, 2018

ashcherbakov commented Jun 29, 2018

ashcherbakov commented Jun 29, 2018

ashcherbakov commented Jun 29, 2018

ashcherbakov commented Jul 2, 2018