-
Notifications
You must be signed in to change notification settings - Fork 25.4k
Use logs dir as working directory #124966
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
Conversation
In the unexpected case that Elasticsearch dies due to a segfault or other similar native issue, a core dump is useful in diagnosing the problem. Yet core dumps are written to the working directory, which is read-only for most installations of Elasticsearch. This commit changes the working directory to the logs dir which should always be writeable.
Hi @rjernst, I've created a changelog YAML for you. |
Pinging @elastic/es-core-infra (Team:Core/Infra) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
...t/groovy/org/elasticsearch/gradle/internal/test/rest/LegacyYamlRestTestPluginFuncTest.groovy
Show resolved
Hide resolved
|
||
ReplacementKey heapDumpPathSub; | ||
if (version.before("8.19.0") && version.onOrAfter("6.3.0")) { | ||
heapDumpPathSub = new ReplacementKey("-XX:HeapDumpPath=data", ""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think using ""
as fallback is wrong here, every string will contain this fallback leading to unexpected and clearly wrong results when replacing... should this be null
or the fallback optional to make the intend clearer in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like your suggestion. I pushed 43db0f2
|
||
ReplacementKey heapDumpPathSub; | ||
if (version.before("8.19.0") && version.onOrAfter("6.3.0")) { | ||
heapDumpPathSub = new ReplacementKey("-XX:HeapDumpPath=data", ""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as mentioned previously, the ""
fallback looks troublesome. I think that needs special handling (and should be more explicit)
💔 Backport failed
You can use sqren/backport to manually backport by running |
@rjernst With this change I am seeing a difference in behavior. Previously when invoking elasticsearch with What is the expected behavior for ES when Here is a stack trace that seems to point to the recent change in ES:
|
With the change to using the logs dir as the working dir of the Elasticsearch process we need to ensure the logs dir exists within the CLI instead of later during startup. relates elastic#124966
With the change to using the logs dir as the working dir of the Elasticsearch process we need to ensure the logs dir exists within the CLI instead of later during startup. relates #124966
In the unexpected case that Elasticsearch dies due to a segfault or other similar native issue, a core dump is useful in diagnosing the problem. Yet core dumps are written to the working directory, which is read-only for most installations of Elasticsearch. This commit changes the working directory to the logs dir which should always be writeable.
With the change to using the logs dir as the working dir of the Elasticsearch process we need to ensure the logs dir exists within the CLI instead of later during startup. relates elastic#124966
In the unexpected case that Elasticsearch dies due to a segfault or other similar native issue, a core dump is useful in diagnosing the problem. Yet core dumps are written to the working directory, which is read-only for most installations of Elasticsearch. This commit changes the working directory to the logs dir which should always be writeable.