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

Die with dignity #19272

Merged
merged 4 commits into from
Jul 7, 2016
Merged

Die with dignity #19272

merged 4 commits into from
Jul 7, 2016

Conversation

jasontedor
Copy link
Member

@jasontedor jasontedor commented Jul 5, 2016

Today when a thread encounters a fatal unrecoverable error that
threatens the stability of the JVM, Elasticsearch marches on. This
includes out of memory errors, stack overflow errors and other errors
that leave the JVM in a questionable state. Instead, the Elasticsearch
JVM should die when these errors are encountered. This commit causes
this to be the case.

Relates #19231

@rmuir
Copy link
Contributor

rmuir commented Jul 5, 2016

+1

Today when a thread encounters a fatal unrecoverable error that
threatens the stability of the JVM, Elasticsearch marches on. This
includes out of memory errors, stack overflow errors and other errors
that leave the JVM in a questionable state. Instead, the Elasticsearch
JVM should die when these errors are encountered. This commit causes
this to be the case.
@tlrx
Copy link
Member

tlrx commented Jul 5, 2016

LGTM, I like it

@tlrx
Copy link
Member

tlrx commented Jul 5, 2016

We might want to document the exit codes somewhere too

@jasontedor
Copy link
Member Author

We might want to document the exit codes somewhere too

@tlrx I think that's a good idea, but I'm unsure of a good place to document it. Do you or @clintongormley have any ideas?

@dadoonet
Copy link
Member

dadoonet commented Jul 5, 2016

FYI we documented something similar for plugins: https://www.elastic.co/guide/en/elasticsearch/plugins/current/_other_command_line_parameters.html

@clintongormley
Copy link
Contributor

@jasontedor not perfect, but perhaps a section under Setup?

This commit adds docs for the Elasticsearch shutdown process,
distinguishing between a clean shutdown and an unclean halt due to fatal
error.
If you're running Elasticsearch directly, you can stop Elasticsearch by sending control-C if you're
running Elasticsearch in the console, or by sending `SIGTERM` to the Elasticsearch process on a
POSIX system. You can obtain the PID to send the signal to via various tools (e.g., `ps` or `jps`)
or by specifying a location to write a PID file to on startup (`-p <path>`).
Copy link
Member

Choose a reason for hiding this comment

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

When ran directly the pid is shown in the logs/stdout too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @tlrx, I pushed 94ef28b.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I should be more clear. I think we can rephrase like:

Elasticsearch indicates the PID in logs or standard ouput during startup:
[INFO ][node ] [Araña] version[2.3.3], pid[6674], build[218bdf1/2016-05-17T15:40:04Z]
You can also obtain...

or something like this

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed 4550140. Is that more like what you had in mind?

Copy link
Member

Choose a reason for hiding this comment

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

4550140 is good, thanks!

This commit adds a note to the Elasticsearch stopping docs that
indicates that the Elasticsearch PID is also available from the
Elasticsearch startup logs.
@jasontedor
Copy link
Member Author

@tlrx @clintongormley I pushed docs in 74c1708; could you take a look?

@tlrx
Copy link
Member

tlrx commented Jul 7, 2016

LGTM

@jasontedor jasontedor merged commit e86aa29 into elastic:master Jul 7, 2016
@jasontedor jasontedor deleted the die-with-dignity branch July 7, 2016 18:44
@clintongormley
Copy link
Contributor

Sorry I'm late to this. You can also get the PID from the node info API:

curl -XGET "http://192.168.1.10:9200/_nodes/process"

{
  "_nodes": {
    "total": 1,
    "successful": 1,
    "failed": 0
  },
  "cluster_name": "elasticsearch",
  "nodes": {
    "uOPHTfzUQUyGRuxdSUBniQ": {
      "name": "Karl Lykos",
      "transport_address": "127.0.0.1:9300",
      "host": "127.0.0.1",
      "ip": "127.0.0.1",
      "version": "5.0.0-alpha4",
      "build_hash": "3f5b994",
      "http_address": "127.0.0.1:9200",
      "roles": [
        "master",
        "data",
        "ingest"
      ],
      "process": {
        "refresh_interval_in_millis": 1000,
        "id": 26283,  ##### HERE #####
        "mlockall": false
      }
    }
  }
}

@jasontedor
Copy link
Member Author

@clintongormley Yes, and the cat nodes API too. The concern I have there is that that will list all the nodes in the cluster when only the local node is needed and that leads to more effort on the end-user? I'm happy to add these though if you think otherwise. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants