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

Fix worker_pid handling by replacing old pid with new one. fix #1723 #1739

Merged
merged 1 commit into from
Nov 9, 2017
Merged

Fix worker_pid handling by replacing old pid with new one. fix #1723 #1739

merged 1 commit into from
Nov 9, 2017

Conversation

repeatedly
Copy link
Member

Old implementation includes dead process pid in worker_pid list even if the process is restarted.
This patch replaces dead process pid with new process pid.

@repeatedly repeatedly requested a review from mururu November 7, 2017 22:45
@repeatedly repeatedly self-assigned this Nov 7, 2017
@repeatedly
Copy link
Member Author

@mururu @abicky Could you check this patch?

abicky
abicky approved these changes Nov 8, 2017
View reviewed changes
Copy link
Contributor

@abicky abicky left a comment

Choose a reason for hiding this comment

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

Thank you for fixing #1723! This patch looks good to me 👍
I checked ServerEngine::MultiWorkerServer#keepalive_workers always set the same worker_id.

@repeatedly
Copy link
Member Author

@mururu If there is no problem, I will merge this.

mururu
mururu approved these changes Nov 9, 2017
View reviewed changes
Copy link
Member

@mururu mururu left a comment

Choose a reason for hiding this comment

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

Looks good.

@repeatedly repeatedly merged commit 5c62aab into master Nov 9, 2017
@repeatedly repeatedly deleted the fix-worker_pid-handling branch November 9, 2017 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abicky abicky abicky approved these changes

@mururu mururu mururu approved these changes

Assignees

@repeatedly repeatedly

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@repeatedly @abicky @mururu