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

Set RuntimeDirectory in systemd service #23526

Merged
merged 1 commit into from
Aug 15, 2017
Merged

Set RuntimeDirectory in systemd service #23526

merged 1 commit into from
Aug 15, 2017

Conversation

jordansissel
Copy link
Contributor

This instruction tells systemd to create a directory /var/run/elasticsearch before starting Elasticsearch.

Without this change, the default PID_DIR (/var/run/elasticsearch) may not exist, and without it, Elasticsearch will fail to start.

@jordansissel
Set RuntimeDirectory
75fe510 
This instruction tells systemd to create a directory /var/run/elasticsearch before starting Elasticsearch.

Without this change, the default PID_DIR (/var/run/elasticsearch) may not exist, and without it, Elasticsearch will fail to start.
@jordansissel jordansissel changed the title Set RuntimeDirectory Set RuntimeDirectory in systemd service Mar 9, 2017
@jordansissel
Copy link
Contributor Author

jordansissel commented Mar 9, 2017 •
edited
Loading

/cc @jpcarey who described the problem to me.

I was able to reproduce[*] the problem he described that /var/run/elasticsearch may not exist, and systemd was not creating it, and the elasticsearch service would fail to start without the directory.

[*] My reproduction was not with elasticsearch's systemd service, but with a much simpler one to test systemd's behavior with respect to /var/run. Without RuntimeDirectory=foo the /var/run/foo directory is not created by systemd.

@jasontedor
Copy link
Member

This directory is created during installation, so I'm not sure I understand how this situation could have arisen?

$ rpm -qlp elasticsearch-5.2.2.rpm  | grep /var/run/elasticsearch
warning: elasticsearch-5.2.2.rpm: Header V4 RSA/SHA512 Signature, key ID d88e42b4: NOKEY
/var/run/elasticsearch

$ dpkg -c elasticsearch-5.2.2.deb  | grep /var/run/elasticsearch
drwxr-xr-x elasticsearch/elasticsearch 0 2017-02-24 17:29 ./var/run/elasticsearch/

@jordansissel
Copy link
Contributor Author

jordansissel commented Mar 9, 2017 via email

@jasontedor
Copy link
Member

We provide /usr/lib/tmpfiles.d/elasticsearch.conf that creates this file on system boot exactly for this reason. I still do not see how this could have arisen? 😦

@rjernst
Copy link
Member

rjernst commented Jun 9, 2017

@jordansissel Given the existence of the tmpfiles configuration @jasontedor mentioned, can this PR be closed?

@jpcarey
Copy link
Contributor

jpcarey commented Jun 9, 2017

I heard, but cannot confirm, that those encountering this did not have the systemd-tmpfiles-setup.service running, which likely meant it was not started at boot for whatever reason.

I had the systemd-tmpfiles-setup.service running, and deleted /var/run/elasticsearch. Elasticsearch would no longer start, as expected (journalctl: Likely root cause: java.nio.file.AccessDeniedException: /var/run/elasticsearch).

If you manually invoke sudo systemd-tmpfiles --create, the /var/run/elasticsearch folder is created.

Rerunning the same test, but adding RuntimeDirectory=elasticsearch to an override file, allowed elasticsearch to start properly when the /var/run/elasticsearch directory was missing.

I would be in favor of adding the RuntimeDirectory, since it seems to avoid needing to rely on an external service's unknown working status / schedule.

https://www.freedesktop.org/software/systemd/man/tmpfiles.d.html

System daemons frequently require private runtime directories below /run to place communication sockets and similar in. For these, consider declaring them in their unit files using RuntimeDirectory= (see systemd.exec(5) for details), if this is feasible.

@dakrone
Copy link
Member

dakrone commented Aug 15, 2017

Given than it's possible systemd-tmpfiles-setup may be a disabled service, I think it is beneficial to have the RuntimeDirectory regardless in the ES service file. Since no one has +1 or -1 this PR I am going to approve and merge it. If anyone disagrees we can definitely revert and re-open discussion, but it has been 2 months so I don't want this to stagnate either way.

@dakrone dakrone merged commit f76fde0 into master Aug 15, 2017
@jasontedor
Copy link
Member

I'm good with this having been integrated after yesterday seeing another user run into a problem with their systemd-tmpfiles-setup not running correctly. See: https://discuss.elastic.co/t/elasticsearch-service-doesnt-start-upon-system-reboot/96994

jasontedor pushed a commit that referenced this pull request Aug 16, 2017
@jordansissel @jasontedor
Set RuntimeDirectory (#23526)
7067f0f 
This instruction tells systemd to create a directory /var/run/elasticsearch before starting Elasticsearch.

Without this change, the default PID_DIR (/var/run/elasticsearch) may not exist, and without it, Elasticsearch will fail to start.
jasontedor pushed a commit that referenced this pull request Aug 16, 2017
@jordansissel @jasontedor
Set RuntimeDirectory (#23526)
6f8466c 
This instruction tells systemd to create a directory /var/run/elasticsearch before starting Elasticsearch.

Without this change, the default PID_DIR (/var/run/elasticsearch) may not exist, and without it, Elasticsearch will fail to start.
@jasontedor
Copy link
Member

I cherry-picked this to 6.0 and 6.x too.

@jasontedor jasontedor added :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts v6.0.0 v6.1.0 v7.0.0 labels Aug 16, 2017
@dakrone
Copy link
Member

dakrone commented Aug 16, 2017

Thanks Jason!

@jasontedor jasontedor mentioned this pull request Aug 16, 2017
Merged
@jasontedor
Copy link
Member

I opened #26229 to add a test too.

@jasontedor jasontedor deleted the systemd-set-runtimedirectory branch August 16, 2017 02:25
@jordansissel
Copy link
Contributor Author

❤️

jasontedor added a commit to glefloch/elasticsearch that referenced this pull request Aug 16, 2017
@jasontedor
Merge branch 'master' into fix/24969
e1bc2ef 
* master: (458 commits)
  Prevent cluster internal `ClusterState.Custom` impls to leak to a client (elastic#26232)
  Add packaging test for systemd runtime directive
  [TEST] Reenable RareClusterStateIt#testDeleteCreateInOneBulk
  Serialize and expose timeout of acknowledged requests in REST layer (elastic#26189)
  (refactor) some opportunities to use diamond operator (elastic#25585)
  [DOCS] Clarified readme for testing a single page
  Settings: Add keystore.seed auto generated secure setting (elastic#26149)
  Update version information (elastic#25226)
  "result" : created -> "result" : "created" (elastic#25446)
  Set RuntimeDirectory (elastic#23526)
  Drop upgrade from full cluster restart tests (elastic#26224)
  Further improve docs for requests_per_second
  Docs disambiguate reindex's requests_per_second (elastic#26185)
  [DOCS] Cleanup link for ec2 discovery (elastic#26222)
  Fix document field equals and hash code test
  Use holder pattern for lazy deprecation loggers
  Settings: Add keystore creation to add commands (elastic#26126)
  Docs: Cleanup docs for ec2 discovery (elastic#26065)
  Fix NPE when `values` is omitted on percentile_ranks agg (elastic#26046)
  Several internal improvements to internal test cluster infra (elastic#26214)
  ...
@lcawl lcawl removed the v6.1.0 label Dec 12, 2017
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
@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
Reviewers

@dakrone dakrone dakrone approved these changes

Assignees
No one assigned
Labels
>bug :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts Team:Delivery Meta label for Delivery team v6.0.0-beta2 v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@jordansissel @jasontedor @rjernst @jpcarey @dakrone @clintongormley @colings86 @mark-vieira @jimczi @lcawl