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

Adding node suspension test and small raft fixes #503

Merged
merged 10 commits into from
Nov 6, 2019

Conversation

olgavrou
Copy link
Contributor

@olgavrou olgavrou commented Nov 1, 2019

Added python test that:

Starts with 3 nodes

  • Initiates timers with a random timeout value from 1-10 seconds for each node
  • Starts sending transactions to the nodes
  • When a timer for a node expires that node process is suspended for a random timeout value between [election_timeout, 3*election_timeout] so it will be sleeping for some time
  • When the new node timer expires, that node process is again resumed

When all the transactions are sent, we add a fourth node and ensure that it can catch up correctly.

The test introduces some fuzziness in the election process with nodes sleeping/waking up at random times and for random intervals.

Regarding Raft:

Resolves #519 #520

I am not adding the new test to the CI before issue #521 is resolved, but this can be reviewed independently and the test added to the CI next.

@olgavrou olgavrou changed the title Adding node suspension test and fixing raft bugs Adding node suspension test and small raft fixes Nov 1, 2019
@ghost
Copy link

ghost commented Nov 1, 2019

images

@jumaffre
Copy link
Contributor

jumaffre commented Nov 1, 2019

I cannot comment on that line directly: In raft.h, when a node recv_request_vote(), should:

auto last_term = get_term_internal(last_idx);

auto answer = (r.last_log_term > last_term) ||
        ((r.last_log_term == last_term) && (r.last_log_idx >= last_idx));

become

auto last_term = get_term_internal(commit_idx);

auto answer = (r.last_log_term > last_term) ||
        ((r.last_log_term == last_term) && (r.last_log_idx >= commit_idx));

We should also probably rename r.last_log_idx to r.commit_idx.

tests/infra/ccf.py Outdated Show resolved Hide resolved
tests/infra/remote.py Outdated Show resolved Hide resolved
tests/node_suspension.py Outdated Show resolved Hide resolved
tests/node_suspension.py Outdated Show resolved Hide resolved
tests/node_suspension.py Outdated Show resolved Hide resolved
tests/node_suspension.py Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Nov 4, 2019

Codecov Report

Merging #503 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #503      +/-   ##
==========================================
+ Coverage   78.21%   78.22%   +0.02%     
==========================================
  Files         140      140              
  Lines       10472    10475       +3     
==========================================
+ Hits         8190     8194       +4     
+ Misses       2282     2281       -1
Flag Coverage Δ
#e2e_BFT 51.08% <0%> (-0.02%) ⬇️
#e2e_CFT 71.85% <100%> (+0.02%) ⬆️
#unit_BFT 65.11% <ø> (ø) ⬆️
#unit_CFT 71.75% <100%> (ø) ⬇️
Impacted Files Coverage Δ
src/consensus/raft/rafttypes.h 91.67% <ø> (ø) ⬆️
src/consensus/raft/raft.h 81.01% <100%> (+0.09%) ⬆️
src/consensus/ledgerenclave.h 81.82% <0%> (+9.09%) ⬆️

@ghost
Copy link

ghost commented Nov 4, 2019

images

@ghost
Copy link

ghost commented Nov 5, 2019

images

src/consensus/raft/rafttypes.h Show resolved Hide resolved
@olgavrou olgavrou marked this pull request as ready for review November 5, 2019 11:55
@olgavrou olgavrou requested a review from a team as a code owner November 5, 2019 11:55
tests/infra/ccf.py Outdated Show resolved Hide resolved
tests/infra/remote.py Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Nov 5, 2019

images

tests/node_suspension.py Outdated Show resolved Hide resolved
@olgavrou olgavrou requested a review from jumaffre November 5, 2019 17:30
@ghost
Copy link

ghost commented Nov 5, 2019

images

@ghost
Copy link

ghost commented Nov 5, 2019

images

@olgavrou olgavrou merged commit 3633020 into master Nov 6, 2019
@olgavrou olgavrou deleted the e2e_integration_test branch November 6, 2019 10:11
eddyashton pushed a commit to eddyashton/CCF that referenced this pull request Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Raft needs to update last_idx and truncate log after rollback
4 participants