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

Add heapdump telemetry device #776

Merged

Conversation

danielmitterdorfer
Copy link
Member

With this commit we add a new telemetry device heapdump that allows
users to capture a heapdump right before a node is shutdown. This can be
useful together with --on-error=abort where we then want to know the
state of the heap at this point.

With this commit we add a new telemetry device `heapdump` that allows
users to capture a heapdump right before a node is shutdown. This can be
useful together with `--on-error=abort` where we then want to know the
state of the heap at this point.
@danielmitterdorfer danielmitterdorfer added enhancement Improves the status quo :Telemetry Telemetry Devices that gather additional metrics discuss Needs further clarification from the team labels Sep 27, 2019
@danielmitterdorfer danielmitterdorfer removed the discuss Needs further clarification from the team label Oct 10, 2019
@danielmitterdorfer danielmitterdorfer added this to the 1.4.0 milestone Oct 10, 2019
@danielmitterdorfer danielmitterdorfer marked this pull request as ready for review October 10, 2019 15:02
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.

Thanks for this useful addition. There's a small bug that needs fixing, but otherwise looks good.

def detach_from_node(self, node, running):
if running:
heap_dump_file = os.path.join(self.log_root, "heap_at_exit_{}.hprof".format(node.pid))
console.info("{}: Writing heap dump to [{}}]".format(self.human_name, heap_dump_file), logger=self.logger)
Copy link
Contributor

Choose a reason for hiding this comment

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

This will error out as [{}]] needs to be [{}]. (Tests also fail)

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've addressed it now in e44cbfe. Can you please have another look?

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

@danielmitterdorfer danielmitterdorfer merged commit c2449ba into elastic:master Oct 25, 2019
@danielmitterdorfer danielmitterdorfer deleted the heapdump-telemetry branch October 25, 2019 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves the status quo :Telemetry Telemetry Devices that gather additional metrics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants