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

cmd/swarm: handle SIGTERM unix signal for clean exit #3649

Merged
merged 1 commit into from Feb 13, 2017

Conversation

Projects
None yet
7 participants
@zelig
Copy link
Contributor

commented Feb 6, 2017

docker container swarm instances used SIGTERM to shutdown
so swarm nodes on the testnet never exited cleanly
therefore they didnt save the syncstate and peers
so everytime they restarted, nodes started syncing from scratch.
as they had the chunks only hashes were spuriously exchanged but that still
likely caused a lot of unnecessary traffic

@zelig zelig added this to the 1.5.9 milestone Feb 6, 2017

@zelig zelig self-assigned this Feb 6, 2017

@mention-bot

This comment has been minimized.

Copy link

commented Feb 6, 2017

@zelig, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fjl, @maran and @nolash to be potential reviewers.

@homotopycolimit

This comment has been minimized.

Copy link
Contributor

commented Feb 6, 2017

as the admin running the swarm testnet cluster, I fully support this PR.

@GitCop

This comment has been minimized.

Copy link

commented Feb 11, 2017

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

  • Commit: 6cf07897b76a0517a824098113973b910fd1b319
  • Commits must be prefixed with the package(s) they modify

Please check the contribution guidelines for more details.


This message was auto-generated by https://gitcop.com

@homotopycolimit homotopycolimit force-pushed the ethersphere:swarm-sigterm-fix branch Feb 11, 2017

@GitCop

This comment has been minimized.

Copy link

commented Feb 11, 2017

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

  • Commit: 53aced33267b634cfade7ddd2ff83530448519c5

  • Commits must be prefixed with the package(s) they modify

  • Commit: 43a9dcd0195b20beff571754fa9567dff50a7470

  • Commits must be prefixed with the package(s) they modify

Please check the contribution guidelines for more details.


This message was auto-generated by https://gitcop.com

@GitCop

This comment has been minimized.

Copy link

commented Feb 11, 2017

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

  • Commit: 53aced33267b634cfade7ddd2ff83530448519c5

  • Commits must be prefixed with the package(s) they modify

  • Commit: 43a9dcd0195b20beff571754fa9567dff50a7470

  • Commits must be prefixed with the package(s) they modify

  • Commit: ba99c034c962e57b827070e71a4221bcf6211833

  • Commits must be prefixed with the package(s) they modify

Please check the contribution guidelines for more details.


This message was auto-generated by https://gitcop.com

@zelig zelig added in progress and removed pr:review labels Feb 11, 2017

@GitCop

This comment has been minimized.

Copy link

commented Feb 11, 2017

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

  • Commit: 53aced33267b634cfade7ddd2ff83530448519c5

  • Commits must be prefixed with the package(s) they modify

  • Commit: 43a9dcd0195b20beff571754fa9567dff50a7470

  • Commits must be prefixed with the package(s) they modify

  • Commit: ba99c034c962e57b827070e71a4221bcf6211833

  • Commits must be prefixed with the package(s) they modify

  • Commit: a0c8e51a52af560aaa9f7b392096c9915f3dd113

  • Commits must be prefixed with the package(s) they modify

Please check the contribution guidelines for more details.


This message was auto-generated by https://gitcop.com

@homotopycolimit homotopycolimit force-pushed the ethersphere:swarm-sigterm-fix branch Feb 11, 2017

@GitCop

This comment has been minimized.

Copy link

commented Feb 11, 2017

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

  • Commit: 53aced33267b634cfade7ddd2ff83530448519c5

  • Commits must be prefixed with the package(s) they modify

  • Commit: 43a9dcd0195b20beff571754fa9567dff50a7470

  • Commits must be prefixed with the package(s) they modify

  • Commit: ba99c034c962e57b827070e71a4221bcf6211833

  • Commits must be prefixed with the package(s) they modify

  • Commit: 9cbdb7c7f2aee7c839cee110ce1c274c0abde62d

  • Commits must be prefixed with the package(s) they modify

Please check the contribution guidelines for more details.


This message was auto-generated by https://gitcop.com

1 similar comment
@GitCop

This comment has been minimized.

Copy link

commented Feb 11, 2017

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

  • Commit: 53aced33267b634cfade7ddd2ff83530448519c5

  • Commits must be prefixed with the package(s) they modify

  • Commit: 43a9dcd0195b20beff571754fa9567dff50a7470

  • Commits must be prefixed with the package(s) they modify

  • Commit: ba99c034c962e57b827070e71a4221bcf6211833

  • Commits must be prefixed with the package(s) they modify

  • Commit: 9cbdb7c7f2aee7c839cee110ce1c274c0abde62d

  • Commits must be prefixed with the package(s) they modify

Please check the contribution guidelines for more details.


This message was auto-generated by https://gitcop.com

@zelig zelig force-pushed the ethersphere:swarm-sigterm-fix branch Feb 12, 2017

@zelig zelig added pr:review and removed in progress labels Feb 12, 2017

<-sigc
glog.V(logger.Info).Infoln("Got sigterm, shutting down...")
stack.Stop()
}()

This comment has been minimized.

Copy link
@fjl

fjl Feb 13, 2017

Contributor

utils.StartNode does a very similar thing. Please consider adding support for it there instead of repeating the same code here.

This comment has been minimized.

Copy link
@homotopycolimit

homotopycolimit Feb 13, 2017

Contributor

I think I just did that. Can you rereview? :)

@homotopycolimit homotopycolimit force-pushed the ethersphere:swarm-sigterm-fix branch 4 times, most recently Feb 13, 2017

@karalabe

This comment has been minimized.

Copy link
Member

commented Feb 13, 2017

@zelig Builds fails.

@homotopycolimit

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2017

@karalabe sorry. that was my fault.
missing import.
fixed.

@homotopycolimit homotopycolimit force-pushed the ethersphere:swarm-sigterm-fix branch to 9ba0793 Feb 13, 2017

@karalabe

This comment has been minimized.

Copy link
Member

commented Feb 13, 2017

@homotopycolimit Thanks, will merge if green.

@zelig zelig force-pushed the ethersphere:swarm-sigterm-fix branch from 9ba0793 to 8883f36 Feb 13, 2017

@karalabe

This comment has been minimized.

Copy link
Member

commented Feb 13, 2017

@zelig @homotopycolimit Ok, so now you force pushed again, resetting all progress and work.

@homotopycolimit

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2017

@karalabe sorry. Did I do something wrong?
I thought I was supposed to use force push so that there is only one commit.
sincere apologies if I caused anyone more work.

What is the correct way to amend a commit?

@karalabe

This comment has been minimized.

Copy link
Member

commented Feb 13, 2017

@zelig @homotopycolimit Both of you are completely unaware that the other is force pushing and both of you are force pushing out each other's work.

@homotopycolimit

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2017

Hey everyone.
Sorry it was all my fault.
I was pushing to Viktor's branch when I shouldn't have been.
for the record, here's where I stopped https://github.com/ethersphere/go-ethereum/tree/swarm-sigterm-fix-aron

@karalabe
Copy link
Member

left a comment

LGTM.

@fjl If you wish, feel free to open a follow up PR that moves this code into cmd/geth.

@karalabe karalabe merged commit 9b16118 into ethereum:master Feb 13, 2017

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
ci/circleci Your tests passed on CircleCI!
Details
commit-message-check/gitcop All commit messages are valid
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@obscuren obscuren removed the pr:review label Feb 13, 2017

@gbalint gbalint deleted the ethersphere:swarm-sigterm-fix branch May 25, 2018

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.