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

Notify systemd when Elasticsearch is ready #44673

Merged
merged 13 commits into from
Jul 24, 2019

Conversation

jasontedor
Copy link
Member

Today our systemd service defaults to a service type of simple. This means that systemd assumes Elasticsearch is ready as soon as the ExecStart (bin/elasticsearch) process is forked off. This means that the service appears ready long before it actually is, so before it is ready to receive requests. It also means that services that want to depend on Elasticsearch being ready to start can not as there is not a reliable mechanism to determine this. This commit changes the service type to notify. This requires that Elasticsearch sends a notification message via libsystemd sd_notify method. This commit does that by using JNA to invoke this native method. Additionally, we use this integration to also notify systemd when we are stopping.

Today our systemd service defaults to a service type of simple. This
means that systemd assumes Elasticsearch is ready as soon as the
ExecStart (bin/elasticsearch) process is forked off. This means that the
service appears ready long before it actually is, so before it is ready
to receive requests. It also means that services that want to depend on
Elasticsearch being ready to start can not as there is not a reliable
mechanism to determine this. This commit changes the service type to
notify. This requires that Elasticsearch sends a notification message
via libsystemd sd_notify method. This commit does that by using JNA to
invoke this native method. Additionally, we use this integration to also
notify systemd when we are stopping.
@jasontedor jasontedor added >enhancement :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts v8.0.0 v7.4.0 labels Jul 22, 2019
@jasontedor jasontedor requested a review from rjernst July 22, 2019 02:27
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Contributor

@alpar-t alpar-t left a comment

Choose a reason for hiding this comment

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

Would it be worth adding a packaging test to test this integration ?


static {
AccessController.doPrivileged((PrivilegedAction<Object>) () -> {
Native.register(Libsystemd.class, "libsystemd.so.0");
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this fail with the packaged distributions on OS-es that don't support systemd, or does everything in our support matrix support it ?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don’t try to load the native library if the environment variable ES_SD_NOTIFY is not set to “true” and in this PR we do that only in the systemd service integration (see the unit file).

@jasontedor
Copy link
Member Author

Would it be worth adding a packaging test to test this integration ?

I considered the existing packaging tests sufficient, since if this integration is broken due to the service type being set to notfiy service start calls would never return.

I’ll look more carefully if there is deeper testing we can do but I considered the existing packaging tests sufficient.

@jasontedor
Copy link
Member Author

@elasticmachine update branch

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM, one minor comment. Thanks for making this only included in the packages and not archives.

@@ -24,6 +24,10 @@
//// SecurityManager impl:
//// Must have all permissions to properly perform access checks

grant codeBase "${codebase.jna}" {
Copy link
Member

Choose a reason for hiding this comment

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

Can this go below the "Very speciall jar permissions" comment please, with the rest of the jar specific grants? (yes the SM here is a jar specific grant, but the comment above is specific to secure SM)

@jasontedor jasontedor merged commit d558d95 into elastic:master Jul 24, 2019
jasontedor added a commit that referenced this pull request Jul 24, 2019
Today our systemd service defaults to a service type of simple. This
means that systemd assumes Elasticsearch is ready as soon as the
ExecStart (bin/elasticsearch) process is forked off. This means that the
service appears ready long before it actually is, so before it is ready
to receive requests. It also means that services that want to depend on
Elasticsearch being ready to start can not as there is not a reliable
mechanism to determine this. This commit changes the service type to
notify. This requires that Elasticsearch sends a notification message
via libsystemd sd_notify method. This commit does that by using JNA to
invoke this native method. Additionally, we use this integration to also
notify systemd when we are stopping.
@jasontedor jasontedor deleted the sd_notify branch July 24, 2019 05:04
polyfractal pushed a commit to polyfractal/elasticsearch that referenced this pull request Jul 29, 2019
Today our systemd service defaults to a service type of simple. This
means that systemd assumes Elasticsearch is ready as soon as the
ExecStart (bin/elasticsearch) process is forked off. This means that the
service appears ready long before it actually is, so before it is ready
to receive requests. It also means that services that want to depend on
Elasticsearch being ready to start can not as there is not a reliable
mechanism to determine this. This commit changes the service type to
notify. This requires that Elasticsearch sends a notification message
via libsystemd sd_notify method. This commit does that by using JNA to
invoke this native method. Additionally, we use this integration to also
notify systemd when we are stopping.
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 v7.4.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants