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

Crash/Restart tests #343

Merged
merged 12 commits into from Jan 6, 2017
Merged

Crash/Restart tests #343

merged 12 commits into from Jan 6, 2017

Conversation

ebuchman
Copy link
Contributor

  • More tests:
    • Crash all nodes in net and restart them (p2p crash all nodes test #339)
    • Crash a single node at pre-defined break points and restart
      • replace AppHashIsStale by saving an IntermediateState

melekes and others added 9 commits December 21, 2016 19:18
Error #1:

```
Error response from daemon: network with name local_testnet already exists
```

Fixed by stopping and removing local_testnet containers and removing
the network

Error #2:

```
docker: Error response from daemon: Conflict. The name "/test_container_basic" is already in use by container a7cd15d479a964675e7f259de4ed852e7dfef85b447514728f437cd0b980a709. You have to remove (or rename) that container to beable to reuse that name..
```

Fixed by adding `--rm` flag.
@codecov-io
Copy link

codecov-io commented Dec 23, 2016

Current coverage is 49.59% (diff: 47.61%)

No coverage report found for unstable at e5fb681.

Powered by Codecov. Last update e5fb681...bae0bc0

@ebuchman ebuchman force-pushed the restart_test branch 3 times, most recently from b127c99 to 0085a73 Compare December 23, 2016 01:51
@ebuchman ebuchman changed the title Restart test Crash/Restart tests Dec 23, 2016
Copy link
Contributor

@jaekwon jaekwon left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -362,6 +362,14 @@ func (cs *ConsensusState) OnStart() error {
// let's go for it anyways, maybe we're fine
}

// If the latest block was applied in the tmsp handshake,
Copy link
Contributor

Choose a reason for hiding this comment

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

The only time we don’t write the current #height to the wal should be if we saved a new block, then called ApplyBlock on it, and failed to complete updateToState(), right?

In state/execution.go, if h.state.LastBlockHeight == appBlockHeight, store is ahead of app but core's state height is at apps height.

... then we apply the block… but maybe we shouldn’t be doing that. That seems like something the cs.wal should complete

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, we want the cswal to actually handle all the cases: #352

@ebuchman ebuchman merged commit 12d92fd into unstable Jan 6, 2017
@ebuchman ebuchman deleted the restart_test branch January 6, 2017 19:55
faddat referenced this pull request in notional-labs/tendermint Apr 3, 2023
Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Lasaro <lasaro@informal.systems>
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.

None yet

4 participants