Skip to content
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.

Sign up for GitHub

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

Jump to bottom

INDY-1946: Restore current 3PC state from audit ledger [skip ci] #1096

Merged
merged 140 commits into from
Apr 1, 2019
Merged

INDY-1946: Restore current 3PC state from audit ledger [skip ci] #1096

merged 140 commits into from
Apr 1, 2019

Conversation

ArtObr
Copy link
Contributor

@ArtObr ArtObr commented Feb 22, 2019

No description provided.

@ArtObr
INDY-1946: Primaries restoration
36ba9f0 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
Merge remote-tracking branch 'ascher/audit-ledger' into indy_1946_2
04bc0a9 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
ashcherbakov
ashcherbakov reviewed Feb 23, 2019
View reviewed changes
plenum/server/node.py Show resolved Hide resolved
plenum/server/node.py Show resolved Hide resolved
plenum/server/node.py Show resolved Hide resolved
plenum/server/node.py Outdated Show resolved Hide resolved
plenum/server/node.py Show resolved Hide resolved
plenum/server/node.py Outdated Show resolved Hide resolved
plenum/server/node.py Outdated Show resolved Hide resolved
plenum/server/node.py Outdated Show resolved Hide resolved
plenum/server/node.py Outdated Show resolved Hide resolved
plenum/server/view_change/view_changer.py Show resolved Hide resolved
ashcherbakov
ashcherbakov reviewed Feb 23, 2019
View reviewed changes
plenum/server/node.py Outdated Show resolved Hide resolved
ArtObr and others added 16 commits February 26, 2019 12:20
@ArtObr
INDY-1946: Review fixes
4a5509d 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
Merge remote-tracking branch 'base/master' into indy_1946_2
f3f4188 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
Flake8 fix
2bb85f4 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
INDY-1946: Logic and test fixes
7178366 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
INDY-1946: View change fix
54fb2f5 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@Toktar
INDY-1946: add integration tests
c5d9cf6 
Signed-off-by: toktar <renata.toktar@dsr-corporation.com>
@ashcherbakov
Removing txn_seq_range_to_3phase_key (in progress)
db7efc6 
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>
@ashcherbakov
Merge branch 'indy_1946_2' of github.com:ArtObr/indy-plenum into indy…
3295bac 
…-1946
@ashcherbakov
fix audit txn
5ab35cd 
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>
@ashcherbakov
Remove txn_seq_range_to_3phase_key
d779d39 
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>
@ashcherbakov
fix comment
1e68962 
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>
@ashcherbakov
Merge branch 'indy_1946_2' of github.com:ArtObr/indy-plenum into indy…
7780e17 
…-1946
@ashcherbakov
fix filling primaries in audit txn
313ecd5 
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>
@ashcherbakov
fix get_by_seq_no for uncommitted txns in ledger
977ee3e 
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>
@ashcherbakov
fix audit freshness test
406ea78 
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>
@ashcherbakov
fix audit freshness test
8ebbf05 
Signed-off-by: ashcherbakov <alexander.sherbakov@dsr-corporation.com>
ArtObr and others added 10 commits March 18, 2019 09:44
@ArtObr
INDY-1946: Test fixes
fbee22d 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
Merge remote-tracking branch 'base/master' into indy_1946_2
2407a9c 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
INDY-1946: Remove useless 3pc markers
a579f31 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
INDY-1946: Test fixes
6e8957d 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
[INDY-1946] fix more checkooints tests
4ee4db4 
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
@ArtObr
Merge pull request #16 from anikitinDSR/public/indy-1946
9356522 
[INDY-1946] fix more checkoints tests
[INDY-1946] fix parametrized test into checkpoint
56ef452 
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
@ArtObr
Merge pull request #17 from anikitinDSR/public/indy-1946
c8eac29 
[INDY-1946] fix parametrized test into checkpoint
@ArtObr
Merge remote-tracking branch 'base/master' into indy_1946_2
842dba8 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
Merge branch 'indy_1946_2' of github.com:ArtObr/indy-plenum into indy…
e95a2ba 
…_1946_2
@ArtObr
Copy link
Contributor Author

ArtObr commented Mar 20, 2019

Need additional future_primaries integration test, to check correct primaries revert.

@ArtObr
Merge remote-tracking branch 'base/master' into indy_1946_2
0a0b494 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
Flake8 fix
6606483 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
INDY-1946: Fix tests
a37e058 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
INDY-1946: Skip test
8c0ff26 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
Copy link
Contributor Author

ArtObr commented Mar 28, 2019

test this please

@ArtObr
INDY-1946: pp_seq_no_restoration refactoring
f9c2e33 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
INDY-1946: Skip backup 3pc checking
67ea67c 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
Revert "INDY-1946: pp_seq_no_restoration refactoring"
cb2d4a2 
This reverts commit f9c2e33.

Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
Revert "INDY-1946: Skip backup 3pc checking"
3c6e32a 
This reverts commit 67ea67c.

Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
Revert "Revert "INDY-1946: pp_seq_no_restoration refactoring""
f90009a 
This reverts commit cb2d4a2.

Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
INDY-1946: Backup last ordered fix
36c2dfe 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
@ArtObr
INDY-1946: Fix tests
fd3b34f 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
skhoroshavin
skhoroshavin reviewed Apr 1, 2019
View reviewed changes
plenum/common/ledger.py

def get_by_seq_no_uncommitted(self, seq_no):
if seq_no > self.uncommitted_size:
raise KeyError
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are okay with having IndexError insteead of KeyError in case seq_no > self.uncommited_size then this additional check is not needed.

skhoroshavin
skhoroshavin reviewed Apr 1, 2019
View reviewed changes
plenum/server/batch_handlers/audit_batch_handler.py
txn[AUDIT_TXN_PRIMARIES] = current_primaries

# 2. Previous primaries field contains primary list
# If primaries did not changed, we will store seq_no delta
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# If primaries did not changed, we will store seq_no delta
# If primaries did not change, we will store seq_no delta

@ArtObr
Fix tests
ebf6fc8 
Signed-off-by: ArtObr <artemobruchnikov@gmail.com>
skhoroshavin
skhoroshavin reviewed Apr 1, 2019
View reviewed changes
plenum/server/future_primaries_batch_handler.py
if len(self.node_states) == 0:
last_state = self.create_node_state_from_current_node()
else:
last_state = copy.deepcopy(self.node_states[-1])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this could be imlemented in a much more safe (and probably efficient) way if NodeState was immutable NamedTuple instead of mutable class with fields.

ashcherbakov
ashcherbakov approved these changes Apr 1, 2019
View reviewed changes
plenum/server/last_sent_pp_store_helper.py

if replica.isPrimary is not True:
if replica.isPrimary is None:
logger.info("{} ignoring stored {} because it is not primary in instance {}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like error message is incorrect here.

plenum/server/replica.py
# As we've selected new primaries, we need to send 3pc batch,
# so this primaries can be saved in audit ledger
if not sent_batches and self.node.primaries_batch_needed:
self.logger.debug("Sending a 3PC batch to propagate newly selected primaries")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to log it as an INFO.

plenum/test/checkpoints/test_checkpoints_removal_after_catchup.py
node.allLedgersCaughtUp()

assert len(replica.checkpoints) == 0
assert len(replica.stashedRecvdCheckpoints) == 0


def test_checkpoints_not_removed_on_backup_primary_replica_after_catchup(
def test_checkpoints_removed_on_backup_primary_replica_after_catchup(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it removed now?

plenum/test/node_catchup/helper.py
@@ -38,7 +39,7 @@ def chk_ledger_and_state(first_node, second_node, ledger_id):
else:
logger.debug("Excluding check_last_ordered_3pc check")

if not exclude_from_check or 'check_last_ordered_3pc_backup' not in exclude_from_check:
if include_in_check and 'check_last_ordered_3pc_backup' in include_in_check:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any tests where we test/expect equal 3PC on backups?

@ashcherbakov ashcherbakov merged commit 35bc655 into hyperledger:master Apr 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Toktar Toktar Toktar left review comments

@skhoroshavin skhoroshavin skhoroshavin approved these changes

@ashcherbakov ashcherbakov ashcherbakov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ArtObr @skhoroshavin @Toktar @ashcherbakov