[INDY-1928] Add freshness info into validator-info output #1058
Conversation
This file contains 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
added 3 commits
January 18, 2019 20:08
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
| @@ -435,7 +435,7 @@ def register_ledger(self, ledger_id): | |||
| if ledger_id not in self.requestQueues: | |||
| self.requestQueues[ledger_id] = OrderedSet() | |||
| self._freshness_checker.register_ledger(ledger_id=ledger_id, | |||
| initial_time=self.get_current_time()) | |||
There was a problem hiding this comment.
get_current_time is used in unit tests to mock the time
There was a problem hiding this comment.
So, we need to
- Create a new field
get_time_for_3pc_batchwhich can be overridden in tests similar toget_current_timeand isnode.utc_epoch()by default - Use
get_time_for_3pc_batchwhen creating a PrePrepare, and when checking for freshness (self._freshness_checker.check_freshness(self.get_time_for_3pc_batch())) - Update all freshness unit tests to patch
get_time_for_3pc_batchinstead ofget_current_time.
plenum/server/validator_info_tool.py
Outdated
|
|
||
| def _is_updated_time_acceptable(self, updated_time): | ||
| current_time = self._node.utc_epoch() | ||
| return current_time - updated_time <= 2 * self._node.config.STATE_FRESHNESS_UPDATE_INTERVAL |
There was a problem hiding this comment.
Please make 2 a parameter in config as well (something like ACCEPTABLE_FRESHNESS_INTERVALS_COUNT)
| @@ -127,6 +127,10 @@ def test_node_info_section(info, node): | |||
| assert len(info['Node_info']['Catchup_status']['Last_txn_3PC_keys']) == 3 | |||
| assert info['Node_info']['Catchup_status']['Ledger_statuses'] | |||
| assert len(info['Node_info']['Catchup_status']['Ledger_statuses']) == 3 | |||
| for idx, ls in info['Node_info']['Catchup_status']['Ledger_statuses'].items(): | |||
There was a problem hiding this comment.
I'm not sure that having information about freshness in 'Catchup_status' is a good idea. Maybe we should create a separate section for this? For example, Freshness_status?
| @@ -435,7 +435,7 @@ def register_ledger(self, ledger_id): | |||
| if ledger_id not in self.requestQueues: | |||
| self.requestQueues[ledger_id] = OrderedSet() | |||
| self._freshness_checker.register_ledger(ledger_id=ledger_id, | |||
| initial_time=self.get_current_time()) | |||
There was a problem hiding this comment.
So, we need to
- Create a new field
get_time_for_3pc_batchwhich can be overridden in tests similar toget_current_timeand isnode.utc_epoch()by default - Use
get_time_for_3pc_batchwhen creating a PrePrepare, and when checking for freshness (self._freshness_checker.check_freshness(self.get_time_for_3pc_batch())) - Update all freshness unit tests to patch
get_time_for_3pc_batchinstead ofget_current_time.
added 3 commits
January 22, 2019 16:10
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
| @@ -435,7 +435,7 @@ def register_ledger(self, ledger_id): | |||
| if ledger_id not in self.requestQueues: | |||
| self.requestQueues[ledger_id] = OrderedSet() | |||
| self._freshness_checker.register_ledger(ledger_id=ledger_id, | |||
| initial_time=self.get_current_time()) | |||
| initial_time=self.get_time_for_3pc_batch()) | |||
There was a problem hiding this comment.
Please use get_time_for_3pc_batch for PrePrepare creation
plenum/server/replica.py
Outdated
| if self._freshness_checker: | ||
| return self._freshness_checker.get_last_update_time() | ||
|
|
||
| def get_time_for_3pc_batch(self)->int: |
There was a problem hiding this comment.
This is a matter of taste, but I prefer to have this as a field in constructor that can be overridden. This makes the tests more clean from my opinion.
added 3 commits
January 23, 2019 10:00
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
ashcherbakov
approved these changes
Jan 23, 2019
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.
{
0: {'Last_updated_time': '2019-01-22 13:27:44+00:00', 'Has_write_consensus': True},
1: {'Last_updated_time': '2019-01-22 13:27:44+00:00', 'Has_write_consensus': True},
2: {'Last_updated_time': '2019-01-22 13:27:44+00:00', 'Has_write_consensus': True}}
}