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

Ensure telemetry path exists for heapdump #1472

Merged
merged 1 commit into from
Apr 11, 2022
Merged

Ensure telemetry path exists for heapdump #1472

merged 1 commit into from
Apr 11, 2022

Conversation

DJRickyB
Copy link
Contributor

@DJRickyB DJRickyB commented Apr 4, 2022

Fixes No such file or directory for default heapdump telemetry

@DJRickyB DJRickyB added the bug Something's wrong label Apr 4, 2022
@DJRickyB DJRickyB added this to the 2.5.0 milestone Apr 4, 2022
@DJRickyB DJRickyB self-assigned this Apr 4, 2022
DJRickyB
DJRickyB commented Apr 11, 2022
View reviewed changes
esrally/telemetry.py
@@ -324,6 +324,7 @@ def __init__(self, log_root):

def detach_from_node(self, node, running):
if running:
io.ensure_dir(self.log_root)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

see JitCompiler.instrument_java_opts and Gc.instrument_java_opts for analogous usage

@dliappis
Copy link
Contributor

I am curious, given that the heapdump is placed under the race directory (e.g. at end of a fresh race I see /home/dl/.rally/benchmarks/races/51a83334-9e7c-4050-b30b-191013b3b696/rally-node-0/telemetry/heap_at_exit_151007.hprof), what are the occasions where this can happen?

@dliappis
Copy link
Contributor

dliappis commented Apr 11, 2022 •
edited
Loading

I am curious, given that the heapdump is placed under the race directory (e.g. at end of a fresh race I see /home/dl/.rally/benchmarks/races/51a83334-9e7c-4050-b30b-191013b3b696/rally-node-0/telemetry/heap_at_exit_151007.hprof), what are the occasions where this can happen?

Oh I think I see the issue! When I run a test benchmark it created an heapdump directory and nothing more. I am going to test the PR now.

dliappis
dliappis approved these changes Apr 11, 2022
View reviewed changes
Copy link
Contributor

@dliappis dliappis left a comment

Choose a reason for hiding this comment

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

LGTM thanks for fixing this bug.

@DJRickyB DJRickyB merged commit 2172986 into elastic:master Apr 11, 2022
@DJRickyB DJRickyB deleted the heapdump_path branch April 11, 2022 13:37
@pquentin pquentin changed the title ensure telemetry path exists for heapdump Ensure telemetry path exists for heapdump Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dliappis dliappis dliappis approved these changes

@michaelbaamonde michaelbaamonde Awaiting requested review from michaelbaamonde

Assignees

@DJRickyB DJRickyB

Labels
bug Something's wrong
Projects
None yet
Milestone
2.5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@DJRickyB @dliappis