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.

Sign up for GitHub

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

Jump to bottom

HotRod: pool: Stop all the workers by closing the stop channel #1453

Closed
mfrw opened this issue Apr 3, 2019 · 2 comments · Fixed by #1454
Closed

HotRod: pool: Stop all the workers by closing the stop channel #1453

mfrw opened this issue Apr 3, 2019 · 2 comments · Fixed by #1454

Comments

@mfrw
Copy link
Contributor

mfrw commented Apr 3, 2019 •
edited
Loading

Requirement - what kind of business use case are you trying to solve?

Instead of sending a signal on the stop chan, closing the stop chan is the correct way to stop all the workers; Reading from a closed chan is always successful.

Problem - what in Jaeger blocks you from solving the requirement?

Proposal - what do you suggest to solve the problem or improve the existing situation?

Close the stop channel instead of sending values to it.

Any open questions to address
mfrw added a commit to mfrw/jaeger that referenced this issue Apr 3, 2019
@mfrw
Fix HotRod job pool termination (jaegertracing#1453)
ba53ce1
@mfrw mfrw mentioned this issue Apr 3, 2019
Merged
mfrw added a commit to mfrw/jaeger that referenced this issue Apr 3, 2019
@mfrw
Fix HotRod job pool termination (jaegertracing#1453)
dffac7c 
Signed-off-by: Muhammad Falak R Wani <[email protected]>
@jpkrohling jpkrohling added the bug label Apr 3, 2019
@yurishkuro yurishkuro removed the bug label Apr 3, 2019
@yurishkuro
Copy link
Member

not a bug as it's not causing any issues.

@mfrw
Copy link
Contributor Author

mfrw commented Apr 17, 2019

@yurishkuro sorry for the late reply,
IMHO, this is a bug. Assuming you have more than 1 worker, calling stop only stops 1 of the workers and not all, while as closing the chan stops all the workers. Please let me know if you still want me to rename the PR.

yurishkuro pushed a commit that referenced this issue Apr 17, 2019
@mfrw @yurishkuro
Fix HotRod job pool termination (#1453) (#1454)
be6340f 
Signed-off-by: Muhammad Falak R Wani <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix HotRod job pool termination (#1453) mfrw/jaeger
3 participants
@jpkrohling @yurishkuro @mfrw