INDY-2140: Cleanup old view change logic #1357
Conversation
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
skhoroshavin
requested review from
lampkin-diet,
ashcherbakov and
Toktar
as code owners
|
This pull request introduces 1 alert when merging e5ec4a4 into 974229a - view on LGTM.com new alerts:
|
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
|
This pull request fixes 3 alerts when merging 0cd6903 into d24bdf2 - view on LGTM.com fixed alerts:
|
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
|
This pull request introduces 3 alerts and fixes 5 when merging e12f3d6 into d24bdf2 - view on LGTM.com new alerts:
fixed alerts:
|
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
|
This pull request fixes 9 alerts when merging 731dc2b into d24bdf2 - view on LGTM.com fixed alerts:
|
|
@ashcherbakov Please, review it too. |
| # TODO: Check whether these still need to be called somewhere after view change: | ||
| # - self.provider.select_primaries() | ||
| # - self.provider.notify_view_change_complete() | ||
| # - self.instance_changes.remove_view(self.view_no) |
There was a problem hiding this comment.
This is already called by new code in _process_new_view_accerted (BTW there is a typo there)
| # self.previous_view_no = None | ||
| # self.previous_master_primary = None | ||
| # TODO: Check whether these still need to be called somewhere after view change: | ||
| # - self.provider.select_primaries() |
There was a problem hiding this comment.
As for self.provider.select_primaries() (which is actually Node's select_primaries):
- It's called from
select_primaries_on_catchup_completewhen audit ledger is empty. Not the case anymore? - It calls
_setup_for_non_master_after_view_change- looks like we may need it since it does some cleanup! - It calls
primary_selectedwhich resets some scheduled actions - need to have a look at this!
| @@ -64,33 +64,3 @@ def txnPoolNodeSet(txnPoolNodeSet, looper, sdk_pool_handle, sdk_wallet_steward, | |||
| node.monitor.isMasterDegraded = lambda: False | |||
|
|
|||
| return txnPoolNodeSet | |||
|
|
|||
|
|
|||
| def test_new_node_accepts_chosen_primary( | |||
There was a problem hiding this comment.
Should we really remove the test? The test case looks valid .
Signed-off-by: Sergey Khoroshavin sergey.khoroshavin@dsr-corporation.com