You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "suvodeep-pyne (via GitHub)" <gi...@apache.org> on 2026/02/12 00:55:14 UTC
[PR] Fix DirectOOMHandler string matching bug that missed Netty direct memory OOM [pinot]
suvodeep-pyne opened a new pull request, #17684:
URL: https://github.com/apache/pinot/pull/17684
## Summary
- Remove fragile string check (`"direct buffer"`) from `DirectOOMHandler.exceptionCaught()` and use `instanceof OutOfMemoryError` alone
- Netty's `OutOfDirectMemoryError` message says `"direct memory"`, not `"direct buffer"` — the check never matched, causing the handler to miss every Netty-triggered direct memory OOM
- When missed, the error fell through to `DataTableHandler` which logged but did not close the corrupted channel, leaving it open but permanently unable to decode responses (TCP byte stream desynchronized)
- `instanceof OutOfMemoryError` without string matching is safe because only direct memory OOM reaches this handler: the upstream frame decoder uses `PooledByteBufAllocator(preferDirect=true)`, and heap OOM from the downstream `DataTableHandler` never propagates backwards in Netty's exception chain
## Test plan
- [x] Unit test: Netty direct memory OOM (`"failed to allocate ... direct memory"`) is caught
- [x] Unit test: JVM direct buffer OOM (`"Direct buffer memory"`) is caught
- [x] Unit test: Non-OOM exceptions are propagated via `ctx.fireExceptionCaught()`
- [ ] Existing tests pass (no regression)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix DirectOOMHandler string matching bug that missed Netty direct memory OOM [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #17684:
URL: https://github.com/apache/pinot/pull/17684
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix DirectOOMHandler string matching bug that missed Netty direct memory OOM [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on PR #17684:
URL: https://github.com/apache/pinot/pull/17684#issuecomment-3890437178
> Netty's OutOfDirectMemoryError message says "direct memory", not "direct buffer"
I think the message is different depending on whether Netty is using its own allocation or not (which depends on whether [io.netty.tryReflectionSetAccessible](https://github.com/netty/netty/blob/15ce81310da0b867554aa3f756f40fc988f062b8/common/src/main/java/io/netty/util/internal/PlatformDependent0.java#L1147) is set or not).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix DirectOOMHandler string matching bug that missed Netty direct memory OOM [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #17684:
URL: https://github.com/apache/pinot/pull/17684#issuecomment-3888208025
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/17684?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 55.61%. Comparing base ([`5be01ef`](https://app.codecov.io/gh/apache/pinot/commit/5be01efb69378530ace34307d4f08be5c1a8f435?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ba5bdb9`](https://app.codecov.io/gh/apache/pinot/commit/ba5bdb9ff9f03111665457b224801945de1f712f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 4 commits behind head on master.
> :exclamation: There is a different number of reports uploaded between BASE (5be01ef) and HEAD (ba5bdb9). Click for more details.
>
> <details><summary>HEAD has 36 uploads less than BASE</summary>
>
>| Flag | BASE (5be01ef) | HEAD (ba5bdb9) |
>|------|------|------|
>|java-21|5|1|
>|unittests1|2|1|
>|unittests|4|1|
>|temurin|10|1|
>|java-11|5|0|
>|unittests2|2|0|
>|integration|6|0|
>|integration2|2|0|
>|integration1|2|0|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #17684 +/- ##
============================================
- Coverage 63.25% 55.61% -7.64%
+ Complexity 1499 720 -779
============================================
Files 3174 2475 -699
Lines 190373 140301 -50072
Branches 29089 22349 -6740
============================================
- Hits 120417 78028 -42389
+ Misses 60619 55687 -4932
+ Partials 9337 6586 -2751
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/17684/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/17684/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/17684/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/17684/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/17684/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/17684/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/17684/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.61% <100.00%> (-7.62%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/17684/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.61% <100.00%> (-7.64%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/17684/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.61% <100.00%> (-7.64%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/17684/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.61% <100.00%> (-0.04%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/17684/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/17684?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org