Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix: stop IPNS republisher ASAP #1976

Merged
merged 8 commits into from
Apr 4, 2019
Merged

fix: stop IPNS republisher ASAP #1976

merged 8 commits into from
Apr 4, 2019

Conversation

alanshaw
Copy link
Member

@alanshaw alanshaw commented Apr 3, 2019

This PR refactors the stop logic to perform stop actions in parallel for various components (where it is safe to do so). This means that libp2p gets stopped at the same time as the republisher. When libp2p stops it closes all connections meaning that the republsher is able to complete a running republish sooner.

Before, we had to wait for republish to complete before stop happens (since there's no way to cancel a DHT put right now).

This PR refactors the stop logic to perform stop actions in parallel for various components (where it is safe to do so). This means that libp2p gets stopped at the same time as the republisher. When libp2p stops it closes all connections meaning that the republsher is able to complete a running republish sooner.

Before, we had to wait for republish to complete before stop happens (since there's no way to cancel a DHT put right now).

License: MIT
Signed-off-by: Alan Shaw <[email protected]>
@ghost ghost assigned alanshaw Apr 3, 2019
@ghost ghost added the status/in-progress In progress label Apr 3, 2019
Copy link
Contributor

@jacobheun jacobheun left a comment

Choose a reason for hiding this comment

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

This looks good, just a note, this will not be a graceful shutdown of the republisher since all libp2p connections will be closed. Any in progress dht connections will also be ended and errored. Since the node is shutting down, this feels reasonable, especially since we have no mechanism to gracefully stop the dht currently.

Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM!

Should you change the commit message to fix: stop ipns republisher asap though? When I read the PR title I was not understanding the goal of the PR.

@alanshaw alanshaw changed the title fix: stop IPNS asap fix: stop ipns republisher asap Apr 3, 2019
@alanshaw alanshaw changed the title fix: stop ipns republisher asap fix: stop IPNS republisher asap Apr 3, 2019
@alanshaw alanshaw changed the title fix: stop IPNS republisher asap fix: stop IPNS republisher ASAP Apr 3, 2019
@alanshaw
Copy link
Member Author

alanshaw commented Apr 3, 2019

wow, did not expect this PR to cause this:

  1) interface-ipfs-core over ipfs-http-client tests
       .key.list
         "after all" hook:
     Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (C:\Users\travis\build\ipfs\js-ipfs\test\node.js)

Alan Shaw added 3 commits April 3, 2019 15:18
License: MIT
Signed-off-by: Alan Shaw <[email protected]>
License: MIT
Signed-off-by: Alan Shaw <[email protected]>
License: MIT
Signed-off-by: Alan Shaw <[email protected]>
@alanshaw alanshaw mentioned this pull request Apr 4, 2019
50 tasks
Alan Shaw added 4 commits April 4, 2019 09:55
License: MIT
Signed-off-by: Alan Shaw <[email protected]>
License: MIT
Signed-off-by: Alan Shaw <[email protected]>
License: MIT
Signed-off-by: Alan Shaw <[email protected]>
@alanshaw alanshaw merged commit 68561c8 into master Apr 4, 2019
@alanshaw alanshaw deleted the fix/ipns-stop branch April 4, 2019 12:00
@ghost ghost removed the status/in-progress In progress label Apr 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants