Skip to content
This repository has been archived by the owner on Aug 23, 2019. It is now read-only.

fix: running queues after stop #322

Closed
wants to merge 1 commit into from

Conversation

alanshaw
Copy link
Member

@alanshaw alanshaw commented Apr 3, 2019

resolves #321

@ghost ghost assigned alanshaw Apr 3, 2019
@ghost ghost added the in progress label Apr 3, 2019
License: MIT
Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
@alanshaw alanshaw force-pushed the fix/running-queues-after-stop branch from 281b2f0 to fae1f15 Compare April 3, 2019 15:21
@jacobheun
Copy link
Contributor

The issue with this current approach is that the switch doesn't currently prohibit dials when it is stopped (which is why some tests are failing, they never start the switch). While libp2p currently prohibits dials when it's offline, it may be something that warrants changing in the future, libp2p/js-libp2p#341.

It might be simpler to just ensure the target queue exists before adding it and starting it, to avoid the change in switch behavior. This is likely an edge case with running queues shutting down while the queues are draining.

@alanshaw alanshaw closed this Apr 3, 2019
@ghost ghost removed the in progress label Apr 3, 2019
@alanshaw
Copy link
Member Author

alanshaw commented Apr 3, 2019

...but if the switch is stopped isn't it reasonable for it to disallow dials? I take the point on libp2p/js-libp2p#341 but should there instead be different modes of operation? e.g. "start in dial only mode"

My issue is that #323 will guard against the edge case but it doesn't actually solve the issue of the node continuing to run after it has been "stopped".

@jacobheun
Copy link
Contributor

My issue is that #323 will guard against the edge case but it doesn't actually solve the issue of the node continuing to run after it has been "stopped".

Yeah sorry, I confused the goal being to just fix #323. I am looking into an issue where there are connections staying open after close, even when the queues are stopped which is likely contributing to this problem.

I do think a configuration of dialOnly or offline is the best route to go for supporting libp2p/js-libp2p#341 properly.

@alanshaw
Copy link
Member Author

alanshaw commented Apr 3, 2019

Yeah sorry, I confused the goal being to just fix #323.

I should have made it clearer. Do you think what's here is valuable? If so, I'm happy to do the work the fix up the tests.

@jacobheun
Copy link
Contributor

Do you think what's here is valuable?

Yes, what if we had the dial manager initialize with this._isRunning set to true instead of false? This would keep the current functionality of allowing offline dials, but once start/stop actions are taken it's no longer considered offline and the dial manager will behave as such.

@alanshaw alanshaw reopened this Apr 4, 2019
@ghost ghost added the in progress label Apr 4, 2019
@alanshaw
Copy link
Member Author

alanshaw commented Apr 4, 2019

Superceded by #324

@alanshaw alanshaw closed this Apr 4, 2019
@ghost ghost removed the 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.

Error on stop
2 participants