Skip to content

cs: exit if SwitchToConsensus fails#3706

Merged
ebuchman merged 2 commits intodevelopfrom
3656-exit-if-switchtoconsensus-fails
Jun 22, 2019
Merged

cs: exit if SwitchToConsensus fails#3706
ebuchman merged 2 commits intodevelopfrom
3656-exit-if-switchtoconsensus-fails

Conversation

@melekes
Copy link
Contributor

@melekes melekes commented Jun 3, 2019 •
edited
Loading

Refs #3656

  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG_PENDING.md

@melekes melekes requested review from ebuchman and xla as code owners June 3, 2019 11:03
@codecov-io
Copy link

codecov-io commented Jun 3, 2019

Codecov Report

Merging #3706 into develop will decrease coverage by 0.28%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##           develop    #3706      +/-   ##
===========================================
- Coverage    63.51%   63.23%   -0.29%     
===========================================
  Files          218      218              
  Lines        18239    18200      -39     
===========================================
- Hits         11585    11508      -77     
- Misses        5693     5714      +21     
- Partials       961      978      +17
Impacted Files Coverage Δ
consensus/reactor.go 70.29% <0%> (-1.9%) ⬇️
privval/signer_validator_endpoint.go 75.55% <0%> (-12.23%) ⬇️
privval/socket_listeners.go 86.2% <0%> (-10.35%) ⬇️
privval/signer_service_endpoint.go 80% <0%> (-9.1%) ⬇️
libs/db/remotedb/remotedb.go 35.89% <0%> (-4.94%) ⬇️
libs/events/events.go 93.2% <0%> (-4.86%) ⬇️
privval/signer_remote.go 78% <0%> (-4%) ⬇️
p2p/pex/pex_reactor.go 80.88% <0%> (-2.36%) ⬇️
consensus/replay.go 70.2% <0%> (-2.05%) ⬇️
... and 2 more

@melekes melekes self-assigned this Jun 3, 2019
@melekes
Copy link
Contributor Author

melekes commented Jun 4, 2019

Not sure if we need to write a test here. I mean we do want to test all failure scenarios in ConsensusState#OnStart, but these should probably be located in consensus_state_test.go.

@thanethomson
Copy link
Contributor

thanethomson commented Jun 4, 2019

Not sure if we need to write a test here. I mean we do want to test all failure scenarios in ConsensusState#OnStart, but these should probably be located in consensus_state_test.go.

It'd be ideal if there was an easy test to write here (e.g. causing the WAL to fail on start), but that may be a bit too contrived? A great integration test would be one that would start a node, kick off a fast sync, and then during the switch-over to consensus cause the ConsensusState WAL to fail.

Where would such an integration test be best placed?

@ebuchman ebuchman mentioned this pull request Jun 22, 2019
Closed
@ebuchman ebuchman merged commit 3e7752c into develop Jun 22, 2019
@ebuchman ebuchman deleted the 3656-exit-if-switchtoconsensus-fails branch June 22, 2019 15:48
@chengwenxi chengwenxi mentioned this pull request Sep 4, 2019
Closed
unclezoro pushed a commit to unclezoro/tendermint that referenced this pull request Sep 6, 2019
@melekes @unclezoro
cs: exit if SwitchToConsensus fails (tendermint#3706)
11cc8d1 
Refs tendermint#3656
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xla xla Awaiting requested review from xla

1 more reviewer

@ebuchman ebuchman ebuchman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@melekes melekes

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@melekes @codecov-io @thanethomson @ebuchman