-
Notifications
You must be signed in to change notification settings - Fork 366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
INDY-2319: use committed node reg when selecting Primaries for the next view #1447
Conversation
ashcherbakov
commented
Dec 26, 2019
- INDY-2319: use committed node when selecting a new Primary in a view
- fixed test_catchup_with_one_slow_node.py
- test cleanup (removed unused methods and tests)
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>
three_pc_batch_view_no = three_pc_batch.view_no if three_pc_batch.original_view_no is None else three_pc_batch.original_view_no | ||
if three_pc_batch_view_no > self._committed_view_no: | ||
self.node_reg_at_beginning_of_view[three_pc_batch_view_no] = list(self.committed_node_reg) | ||
self._committed_view_no = three_pc_batch_view_no |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May this line should be under if
condition? If now is reordering phase after view_change then self.node_reg_at_beginning_of_view[three_pc_batch_view_no]
will be rewritten on each batch and self._committed_view_no
will be dropped to previous view_no.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I agree that it will be more clear to move it under if
condition.
However, even the current code should not lead to any issues, since commit_batch
is called from non-committed (previously non-ordered) batches only. So, if this is re-ordering after view change phase, this will be called only if a batch is not ordered in the previous view. If so, the current _committed_view_no
can not be higher yet as we don't allow gaps in PrePrepares (we can not Commit in the new view until we finish re-ordering of all batches from the previous view).
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>