KAFKA-19932: adding handling of OOM and avoiding wrapped as timeout by iit2009060 · Pull Request #21117 · apache/kafka

iit2009060 · 2025-12-10T08:30:51Z

adding handling of OOM and avoiding wrapped as timeout

iit2009060 · 2025-12-10T08:31:34Z

@kirktrue can you review it

kirktrue

Thanks for the PR @iit2009060. Can you add some unit tests to validate the work? Thanks!

iit2009060 · 2025-12-11T12:54:36Z

Thanks for the PR @iit2009060. Can you add some unit tests to validate the work? Thanks!

added

iit2009060 · 2025-12-16T04:37:27Z

@kirktrue can you review it.

iit2009060 · 2025-12-27T17:35:48Z

@kirktrue can you please review it

kirktrue

@iit2009060 Sorry for the delay. Thanks for the addition of the unit tests.

I'm going back and forth on whether we should handle this outside of handleTimeoutFailure() or not. On the one hand, the root cause (OOM) is not a timeout, but on the other hand, we might consider hitting an OOM to be a "retriable" error. Thoughts?

Also, I provided some minor feedback on the tests.

Thanks!

kirktrue · 2026-01-07T17:48:43Z

clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java

+            // Create a spy of MetadataResponse that throws OutOfMemoryError when topicMetadata() is accessed
+            // The AdminClient calls response.topicMetadata() in listTopics handleResponse(), which will trigger the OOM
+            MetadataResponseData data = new MetadataResponseData();
+            MetadataResponse realResponse = new MetadataResponse(data, ApiKeys.METADATA.latestVersion());
+            MetadataResponse spyResponse = spy(realResponse);


Is it possible to use a generic "mock" here instead of a spy?

kirktrue · 2026-01-07T17:50:49Z

clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java

+            ExecutionException exception = assertThrows(ExecutionException.class, () -> result.names().get());
+            assertInstanceOf(OutOfMemoryError.class, exception.getCause(),
+                    "Expected OutOfMemoryError to be propagated, but got: " + exception.getCause());


Can you see if you can use TestUtils.assertFutureThrows() here? It could be slightly less boilerplate.

kirktrue · 2026-01-07T17:51:27Z

clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java

+            ExecutionException exception = assertThrows(ExecutionException.class, () -> result.names().get());
+            assertInstanceOf(OutOfMemoryError.class, exception.getCause(),
+                    "Expected OutOfMemoryError to be propagated, but got: " + exception.getCause());
+            assertEquals("Simulated OOM during response handling", exception.getCause().getMessage());


Another option is to create the OutOfMemoryError outside of the doThrow() and then perform assertEquals().

kirktrue · 2026-01-15T19:55:10Z

@iit2009060—gentle reminder on this. Thanks!

kirktrue · 2026-01-29T20:45:40Z

@iit2009060—gentle reminder on this. Thanks!

iit2009060 · 2026-02-01T14:36:45Z

@iit2009060—gentle reminder on this. Thanks!

@kirktrue Thanks for reminding me. I will have a look.

kirktrue · 2026-02-17T20:09:01Z

@iit2009060—do you still have the time and motivation to work on this, or should we hand it off to someone else? I just want to make sure we get it into Kafka 4.3. Thanks!

iit2009060 · 2026-02-17T23:59:53Z

@iit2009060—do you still have the time and motivation to work on this, or should we hand it off to someone else? I just want to make sure we get it into Kafka 4.3. Thanks!

i am really sorry if you have to follow up on this multiple times.I have addressed the comments.

iit2009060 · 2026-02-18T00:09:10Z

I'm going back and forth on whether we should handle this outside of handleTimeoutFailure() or not. On the one hand, the root cause (OOM) is not a timeout, but on the other hand, we might consider hitting an OOM to be a "retriable" error. Thoughts?
Yes i think OOM is not a retryable error and we should not be handling in a timeout failure. We should fail fast instead of waiting on the timeout. Should I make a change for this ?

kirktrue · 2026-02-26T19:52:20Z

Yes i think OOM is not a retryable error and we should not be handling in a timeout failure. We should fail fast instead of waiting on the timeout. Should I make a change for this ?

Yes, please.

Nikita-Shupletsov · 2026-02-26T19:57:46Z

clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java

            }
-            if (cause instanceof TimeoutException) {
+            // Don't mask OutOfMemoryError as TimeoutException - propagate it directly
+            if (cause instanceof OutOfMemoryError) {


is there any reason to wrap any errors into a TImeoutException?
I may be wrong, but it looks like it may make sense to treat all errors the same way.
wdyt?

@Nikita-Shupletsov I am moving Out of memory error out of Timeout as out of memory is not a retryable error.

yeah, I understand that, sorry for not being crisp. My question was more like: as we found that problem with OutOfMemoryError, should we treat all errors the same way? I am not sure if StackOverFlowError for example is a retriable error in that context, or NotSuchMethodError.

Yes that should also not be treated a timedout error , we can open a separate JIRA to handle this specific errors to move out of timedout exception.

@iit2009060—Can you create that separate PR?

@kirktrue can we track in a separate ticket? This PR has been open for a long time.

@iit2009060
May I ask what your concern is? it should be a one-liner. I am not 100% sure why we would need a separate ticket for that

@Nikita-Shupletsov Are you suggesting instead of catching OutOfMemoryError , we should catch VirtualMachineError which avoid all other non retryable exception to fall under the timedout error?

effectively yes

@Nikita-Shupletsov @kirktrue, I have made the changes. Please check

iit2009060 · 2026-03-03T11:35:15Z

Yes i think OOM is not a retryable error and we should not be handling in a timeout failure. We should fail fast instead of waiting on the timeout. Should I make a change for this ?

Yes, please.

done

iit2009060 · 2026-03-03T17:06:31Z

@kirktrue Please review it , I have made the changes.

github-actions bot added triage PRs from the community clients small Small PRs labels Dec 10, 2025

DL1231 added the ci-approved label Dec 10, 2025

kirktrue reviewed Dec 10, 2025

View reviewed changes

github-actions bot removed the triage PRs from the community label Dec 11, 2025

iit2009060 force-pushed the KAFKA-19932 branch from 13fa78a to 1011d7a Compare December 11, 2025 12:27

kirktrue reviewed Jan 7, 2026

View reviewed changes

mjsax changed the title ~~KAFKA-19932 adding handling of OOM and avoiding wrapped as timeout~~ KAFKA-19932: adding handling of OOM and avoiding wrapped as timeout Jan 29, 2026

github-actions bot added streams and removed small Small PRs labels Feb 21, 2026

iit2009060 force-pushed the KAFKA-19932 branch from 0d15324 to fe7799f Compare February 21, 2026 17:11

github-actions bot added the small Small PRs label Feb 21, 2026

Nikita-Shupletsov reviewed Feb 26, 2026

View reviewed changes

iit2009060 force-pushed the KAFKA-19932 branch from 110d1a8 to 6febde3 Compare March 3, 2026 16:59

iit2009060 added 2 commits March 3, 2026 23:01

adding handling of OOM and avoiding wrapped as timeout

23ea9c2

adding test case for OOM detection

7d8f193

iit2009060 added 2 commits March 3, 2026 23:01

Converting spy to mock

2813d6b

Update OOM handling outside of timeout

dce9828

iit2009060 force-pushed the KAFKA-19932 branch from 6febde3 to dce9828 Compare March 3, 2026 17:31

Catch VirtualMachineError instead of OOM

238ce9d

iit2009060 force-pushed the KAFKA-19932 branch from 2888280 to 238ce9d Compare March 14, 2026 15:23

Conversation

iit2009060 commented Dec 10, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iit2009060 commented Dec 10, 2025

Uh oh!

kirktrue left a comment

Choose a reason for hiding this comment

Uh oh!

iit2009060 commented Dec 11, 2025

Uh oh!

iit2009060 commented Dec 16, 2025

Uh oh!

iit2009060 commented Dec 27, 2025

Uh oh!

kirktrue left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kirktrue commented Jan 15, 2026

Uh oh!

kirktrue commented Jan 29, 2026

Uh oh!

iit2009060 commented Feb 1, 2026

Uh oh!

kirktrue commented Feb 17, 2026

Uh oh!

iit2009060 commented Feb 17, 2026

Uh oh!

iit2009060 commented Feb 18, 2026

Uh oh!

kirktrue commented Feb 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nikita-Shupletsov Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iit2009060 Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iit2009060 commented Mar 3, 2026

Uh oh!

iit2009060 commented Mar 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

iit2009060 commented Dec 10, 2025 •

edited by github-actions bot

Loading

Nikita-Shupletsov Mar 3, 2026 •

edited

Loading

iit2009060 Mar 11, 2026 •

edited

Loading