[INDY-1478] Fix CurrentState building and FutureViewChangeDone logic #836
Conversation
lampkin-diet
commented
Jul 25, 2018
lampkin-diet
commented
- Now, CurrentState will be built with last_completed_view_no instead of proposed_view_no (view_changer.view_no)
- Devide process initial_propagate_primary and other received CurrentState while node already has a primary
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
…ur as FutureViewChangeDone Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
plenum/server/node.py
Outdated
| @@ -1521,13 +1521,17 @@ def msgHasAcceptableViewNo(self, msg, frm, from_current_state: bool = False) -> | |||
| if self.viewNo - view_no > 1: | |||
| self.discard(msg, "un-acceptable viewNo {}" | |||
| .format(view_no), logMethod=logger.warning) | |||
| elif (view_no > self.viewNo) or (from_current_state and self._should_accept_current_state()): | |||
| elif (view_no > self.viewNo) or from_current_state: | |||
There was a problem hiding this comment.
What if this is a CurrentState for a lower viewno?
| Check, that view_change would be started with quorum of propagate_primary view_change_done messages | ||
| if is_initial_propagate_primary is True | ||
| """ | ||
| quorum = fake_node.quorums.propagate_primary.value |
There was a problem hiding this comment.
It's better to specify the quorum explicitly in the test (to be sure that this is f+1, not occasionally changed to n-f).
| assert not res | ||
|
|
||
|
|
||
| def test_future_vcdone_vc_not_in_progress(fake_node): |
There was a problem hiding this comment.
These two tests can be parametrized with params=['view_change_in_progress', 'view_change_not_in_progress'], can't they?
| nodeCount = 6 | ||
|
|
||
|
|
||
| def test_vc_by_current_state(txnPoolNodeSet, |
There was a problem hiding this comment.
Don't we already have such a test?
|
|
||
| nodeCount = 6 | ||
|
|
||
|
|
There was a problem hiding this comment.
I think the following integration test needs to be added:
Stop node; do view change to v1, do view change to v2 (in progress); start node; make sure that the restarted node is on v1 and not on v2.
| ind_count = len(self._next_view_indications[view_no]) | ||
| if self.quorums.propagate_primary.is_reached(ind_count): | ||
| if self._qourum_is_reached(ind_count, is_initial_propagate_primary=is_initial_propagate_primary): |
There was a problem hiding this comment.
This is the quorum for starting of a view change for Primary propagation.
Please also have a look at quorum property in ViewChanger
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
…he higher than 0 view) Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
| @@ -1521,13 +1521,16 @@ def msgHasAcceptableViewNo(self, msg, frm, from_current_state: bool = False) -> | |||
| if self.viewNo - view_no > 1: | |||
| self.discard(msg, "un-acceptable viewNo {}" | |||
| .format(view_no), logMethod=logger.warning) | |||
| elif (view_no > self.viewNo) or (from_current_state and self._should_accept_current_state()): | |||
| if isinstance(msg, ViewChangeDone) and view_no < self.viewNo: | |||
There was a problem hiding this comment.
Do we need this check?
