INDY-1545: stop processing checkpoints in view change #854
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: toktar <[email protected]>
Changes: - add bugfix for view_no in checkpoint cleaning. Choose view_no from last_ordered_3pc - update test test_checkpoints_removal_in_view_change.py Signed-off-by: toktar <[email protected]>
…nto bugfix-1545-checkponts-cleaning-preprepares
Signed-off-by: toktar <[email protected]>
Toktar
force-pushed
the
bugfix-1545-checkponts-cleaning-preprepares
branch
from
8c20ef3 to
e5c3be7
Compare
Changes: - update checks - update comments Signed-off-by: toktar <[email protected]>
ashcherbakov
reviewed
Aug 6, 2018
plenum/server/replica.py
Outdated
| @@ -1883,8 +1883,9 @@ def markCheckPointStable(self, seqNo): | |||
| for k in previousCheckpoints: | |||
| self.logger.trace("{} removing previous checkpoint {}".format(self, k)) | |||
| self.checkpoints.pop(k) | |||
| self._remove_stashed_checkpoints(till_3pc_key=(self.viewNo, seqNo)) | |||
| self._gc((self.viewNo, seqNo)) | |||
| view_no = self.last_ordered_3pc[0] | |||
There was a problem hiding this comment.
Can we mark checkpoint stable if we don't order anything? In this case last_ordered can be much below than the stable checkpoint's seqNo.
There was a problem hiding this comment.
Is this case reproduce when node in catchup?
There was a problem hiding this comment.
We clean 3pc messages for transactions ordered in catchup in the method _remove_till_caught_up_3pc in catchup
indy-plenum/plenum/server/replica.py
Lines 2557 to 2579 in 3641daf
ashcherbakov
reviewed
Aug 7, 2018
| fast_nodes, | ||
| (0, CHK_FREQ + 1))) | ||
| # check that fast_nodes finalized first checkpoint | ||
| looper.run(eventually(check_checkpoint_finish, |
There was a problem hiding this comment.
Please check that checkpoint is not finalized for slow_nodes
ashcherbakov
reviewed
Aug 7, 2018
| for n in nodes: | ||
| assert (start_pp_seq_no, end_pp_seq_no) in n.master_replica.checkpoints | ||
| ckState = n.master_replica.checkpoints[(start_pp_seq_no, end_pp_seq_no)] | ||
| assert n.master_replica.quorums.checkpoint.is_reached(len(ckState.receivedDigests)) |
Signed-off-by: toktar <[email protected]>
Signed-off-by: toktar <[email protected]>
Toktar
changed the title
Signed-off-by: toktar <[email protected]>
ashcherbakov
reviewed
Aug 8, 2018
| @@ -1782,6 +1782,12 @@ def processCheckpoint(self, msg: Checkpoint, sender: str) -> bool: | |||
| self.discard(msg, reason="Checkpoint already stable", logMethod=self.logger.debug) | |||
| return True | |||
|
|
|||
| if self.isPrimary is None and msg.viewNo < self.viewNo: | |||
There was a problem hiding this comment.
Why do we have a check self.isPrimary is None?
ashcherbakov
approved these changes
Aug 8, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix that a checkpoints finalize cleans necessary data from requests and 3pc queues in view change before catchup.
Changes: