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

Factor out PID file creation and add tests #8775

Merged
merged 1 commit into from Dec 4, 2014

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Dec 4, 2014

This commit factors out the PID file creation from bootstrap and adds
tests for error conditions etc. We also can't rely on DELETE_ON_CLOSE
since it might not even write the file depending on the OS and JVM implementation.
This impl uses a shutdown hook to best-effort remove the pid file if it was written.

Closes #8771

}
this.path = path;
this.deleteOnExit = deleteOnExit;
this.pid = pid;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like it much when constructors have side-effects. Can we maybe move the API from

new PidFile(path, true);

to something like

PidFile pidFile = PidFile.create(path, true);

to make it clear that there is something happening (since there is a verb)

@jpountz
Copy link
Contributor

jpountz commented Dec 4, 2014

@s1monw left a comment

@jpountz jpountz removed the review label Dec 4, 2014
@s1monw
Copy link
Contributor Author

s1monw commented Dec 4, 2014

@jpountz applied fixes for your comments

@jpountz
Copy link
Contributor

jpountz commented Dec 4, 2014

LGTM

This commit factors out the PID file creation from bootstrap and adds
tests for error conditions etc. We also can't rely on DELETE_ON_CLOSE
since it might not even write the file depending on the OS and JVM implementation.
This impl uses a shutdown hook to best-effort remove the pid file if it was written.

Closes elastic#8771
@s1monw s1monw merged commit f4052fd into elastic:master Dec 4, 2014
@s1monw s1monw deleted the issues/8771 branch December 4, 2014 10:13
@clintongormley clintongormley added >enhancement :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts labels Jun 8, 2015
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts >enhancement Team:Delivery Meta label for Delivery team v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: elasticsearch PID file writing fails
4 participants