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

Use a real out of memory error in die with dignity #77039

Merged
merged 4 commits into from
Sep 1, 2021

Conversation

jasontedor
Copy link
Member

@jasontedor jasontedor commented Aug 30, 2021

Today when testing dying with dignity, we simply throw an OutOfMemoryError. We know this should not get caught by any intermediate code and end up in the uncaught exception handler. This allows us to test that this exception handler is able to successfully kill the VM. However, it is on the table to no longer use the uncaught exception handler, but instead the built-in support for ExitOnOutOfMemoryError. A fake OutOfMemoryError would not be processed by this handler, so to prepare the way, we switch to using a real OutOfMemoryError.

Relates #71542

Today when testing dying with dignity, we simply throw an
OutOfMemoryError. We know this should not get caught by any intermediate
code and end up in the uncaught exception handler. This allows us to
test that this exception handler is able to successfully kill the
VM. However, it is on the table to no longer use the uncaught exception
handler, but instead the built-in support for ExitOnOutOfMemoryError. A
fake OutOfMemoryError would not be processed by this handler, so to
prepare the way, we switch to using a real OutOfMemoryError.
@jasontedor jasontedor added :Core/Infra/Core Core issues without another label v8.0.0 v7.16.0 labels Aug 30, 2021
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Aug 30, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

LGTM2, thanks Jason!

@@ -62,7 +62,7 @@ public void testDieWithDignity() throws Exception {
".*ElasticsearchUncaughtExceptionHandler.*",
".*javaRestTest-0.*",
".*fatal error in thread \\[Thread-\\d+\\], exiting.*",
".*java.lang.OutOfMemoryError: die with dignity.*"
".*java.lang.OutOfMemoryError: Requested array size exceeds VM limit.*"
Copy link
Contributor

Choose a reason for hiding this comment

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

This leaks jvm implementation into our tests. I am not really against that, but perhaps we can add a comment to remove the text from matching if it fails?

@jasontedor jasontedor merged commit 3c589ef into elastic:master Sep 1, 2021
jasontedor added a commit that referenced this pull request Sep 1, 2021
Today when testing dying with dignity, we simply throw an
OutOfMemoryError. We know this should not get caught by any intermediate
code and end up in the uncaught exception handler. This allows us to
test that this exception handler is able to successfully kill the
VM. However, it is on the table to no longer use the uncaught exception
handler, but instead the built-in support for ExitOnOutOfMemoryError. A
fake OutOfMemoryError would not be processed by this handler, so to
prepare the way, we switch to using a real OutOfMemoryError.
@jasontedor jasontedor deleted the die-with-dignity-actual-oom branch September 1, 2021 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >non-issue Team:Core/Infra Meta label for core/infra team v7.16.0 v8.0.0-alpha2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants