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.

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

Sync/rejoin lagging-behind lookup nodes after recovery #1726

Merged
merged 11 commits into from
Aug 16, 2019

Conversation

ckyang
Copy link
Contributor

@ckyang ckyang commented Jul 18, 2019

Description

As https://github.com/Zilliqa/Issues/issues/542 mentioned, we need to synchronize after recovery, with the larger and larger size of persistence database.

Backward Compatibility

  • This is not a breaking change
  • This is a breaking change

Review Suggestion

Status

Implementation

  • ready for review

Integration Test (Core Team)

  • local machine test
  • small-scale cloud test

@ckyang ckyang self-assigned this Jul 18, 2019
@ckyang ckyang added this to PRs in development in Core via automation Jul 18, 2019
@codecov-io
Copy link

codecov-io commented Jul 18, 2019

Codecov Report

Merging #1726 into master will decrease coverage by 0.09%.
The diff coverage is 0%.

@@           Coverage Diff            @@
##           master   #1726     +/-   ##
========================================
- Coverage    33.5%   33.4%   -0.1%     
========================================
  Files         270     270             
  Lines       33277   33383    +106     
========================================
+ Hits        11148   11151      +3     
- Misses      22129   22232    +103
Impacted Files Coverage Δ
src/common/MessageNames.h 100% <ø> (ø) ⬆️
src/libLookup/Lookup.h 0% <ø> (ø) ⬆️
src/libLookup/Lookup.cpp 4.46% <0%> (-0.24%) ⬇️
src/libZilliqa/Zilliqa.cpp 4.14% <0%> (-0.03%) ⬇️
src/libNode/FinalBlockProcessing.cpp 0% <0%> (ø) ⬆️
src/libUtils/SWInfo.cpp 93.12% <0%> (+2.29%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8881cbe...7d1dd48. Read the comment docs.

@ckyang ckyang force-pushed the fix/recover_join branch 8 times, most recently from 160067a to 078151e Compare July 30, 2019 06:54
@ckyang ckyang force-pushed the fix/recover_join branch 10 times, most recently from c50a215 to defde84 Compare August 1, 2019 10:01
@ckyang ckyang changed the title Synchronize after recovery Lookup synchronization after recovery Aug 2, 2019
@ckyang ckyang changed the title Lookup synchronization after recovery Lookup sync/rejoin after recovery Aug 2, 2019
@ckyang ckyang changed the title Lookup sync/rejoin after recovery Sync/rejoin lagging-behind lookup nodes after recovery Aug 2, 2019
@ckyang ckyang force-pushed the fix/recover_join branch 3 times, most recently from 7024de6 to d2d1e63 Compare August 2, 2019 08:28
@ckyang ckyang force-pushed the fix/recover_join branch 3 times, most recently from 76db641 to 1279397 Compare August 8, 2019 02:49
@ckyang ckyang force-pushed the fix/recover_join branch 2 times, most recently from 6579492 to fdadad9 Compare August 14, 2019 06:49
@ckyang ckyang added Ready Ready for review and removed in-progress labels Aug 14, 2019
// retMicroBlocks.push_back(*mbptr);
// }
// }
LOG_GENERAL(INFO, "Reques for " << microBlockHashes.size() << " blocks");
Copy link
Contributor

Choose a reason for hiding this comment

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

spell mistake Reques

src/libLookup/Lookup.cpp Show resolved Hide resolved
// vector<TxnHash> txnhashes;
// txnhashes.clear();
vector<TxnHash> txnhashes;
txnhashes.clear();
Copy link
Contributor

Choose a reason for hiding this comment

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

may not need to clear


// bytes setTxnMsg = {MessageType::LOOKUP,
// LookupInstructionType::SETTXNFROMLOOKUP};
vector<TransactionWithReceipt> txnvector;
Copy link
Contributor

Choose a reason for hiding this comment

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

can rename it to txns ??

@ckyang ckyang requested review from ShengguangXiao and removed request for KaustubhShamshery August 16, 2019 03:45
Core automation moved this from PRs in development to PRs approved - ready to merge! Aug 16, 2019
@ansnunez ansnunez merged commit b70d988 into master Aug 16, 2019
Core automation moved this from PRs approved - ready to merge! to PRs done (merged) Aug 16, 2019
@ShengguangXiao ShengguangXiao deleted the fix/recover_join branch September 2, 2019 05:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Ready Ready for review
Projects
Core
  
PRs done (merged)
Development

Successfully merging this pull request may close these issues.

None yet

5 participants