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

Lisk Node failed to start with snapshot mode - Closes #3214 #3218

Merged
merged 1 commit into from Mar 28, 2019

Conversation

Projects
4 participants
@lsilvs
Copy link
Member

commented Mar 28, 2019

What was the problem?

During snapshot config.peers.enabled is set to false preventing to create the webSocket server and crashing when trying to listen.

How did I fix it?

I check if peers is enabled before creating and listening to webSocket server.

How to test it?

node src/index.js -s 1000 -n testnet

Review checklist

  • The PR resolves #3214
  • All new code is covered with unit tests
  • All new code was formatted with Prettier
  • Linting passes
  • Tests pass
  • Commit messages follow the commit guidelines
  • Documentation has been added/updated

@lsilvs lsilvs self-assigned this Mar 28, 2019

@lsilvs lsilvs added this to Pending Review in Version 1.6.0 via automation Mar 28, 2019

@lsilvs lsilvs requested review from nazarhussain and ManuGowda Mar 28, 2019

@ManuGowda

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

@lsilvs tests? I am afraid we had some tests before as @4miners informed me, if not then its good to have a test to check of the snapshot works as expected, at integration level.

@ManuGowda

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

otherwise the PR LGTM 👍

@shuse2

shuse2 approved these changes Mar 28, 2019

@shuse2 shuse2 changed the base branch from development to 1.6.0 Mar 28, 2019

@lsilvs

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

@ManuGowda The integration test is there but it tests the functions directly. This issue was present when starting the application which should be tested in the functional level but it's tricky as it requires starting the network and waiting for at least one round before running the snapshotting process.

@ManuGowda

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

@ManuGowda The integration test is there but it tests the functions directly. This issue was present when starting the application which should be tested in the functional level but it's tricky as it requires starting the network and waiting for at least one round before running the snapshotting process.

I think it will not be a big issue as we a rounds test waiting for multiple rounds, so for this use case we can just wait for 1 round and see if it works fine, my only worry is to avoiding this type issues at the base level so that we don't delay in releases, see what best you can do.

@shuse2 shuse2 merged commit a849ee2 into 1.6.0 Mar 28, 2019

3 checks passed

jenkins-ci/lisk-core This commit looks good
Details
jenkins-ci/lisk-core-network This commit looks good
Details
security/snyk - package.json (LiskHQ) No manifest changes detected

Version 1.6.0 automation moved this from Pending Review to Closed PRs Mar 28, 2019

@shuse2 shuse2 deleted the 3214-lisk-node-fail-snapshot-mode branch Mar 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.