You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "Akanksha-kedia (via GitHub)" <gi...@apache.org> on 2026/05/13 11:36:08 UTC

[PR] [fix] Fix NPE in StreamOp.fetchExistingTotalDocs when exceptions array is present [pinot]

Akanksha-kedia opened a new pull request, #18491:
URL: https://github.com/apache/pinot/pull/18491

   ## Description
   
   Fix a `NullPointerException` in `StreamOp.fetchExistingTotalDocs` that occurs when the broker response contains a non-empty `exceptions` array.
   
   Fixes #18490
   
   ## Root Cause
   
   The `exceptions` field in the broker query response is a JSON **array** (`[{"errorCode": 410, ...}]`), but the code was calling `exceptions.get("errorCode")` as if it were a JSON object. Jackson's `ArrayNode.get(String)` always returns `null`, so the subsequent `errorCode.asInt()` throws an NPE.
   
   ## Changes Made
   
   1. **Fix array access**: Index into `exceptions.get(0)` first, then extract `errorCode` from the first exception object
   2. **Add null guards**: Defensively handle the case where the array element or errorCode field is missing
   3. **Handle BROKER_SEGMENT_UNAVAILABLE (305)**: Treat this as a transient condition (segments still loading after a rolling upgrade) and return 0 to allow the caller's retry loop to continue, consistent with the existing `BROKER_INSTANCE_MISSING` handling
   
   ## Testing Done
   
   - [x] `mvn spotless:check` passes for `pinot-compatibility-verifier`
   - [x] Code review — the fix is minimal and follows the existing pattern in the method
   
   ## Checklist
   
   - [x] Code follows project style guidelines
   - [x] Self-review completed
   - [x] No new warnings introduced
   - [x] Single-concern change (split from #18360 per reviewer feedback)


-- 
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] Fix NPE in StreamOp.fetchExistingTotalDocs when exceptions array is present [pinot]

Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on PR #18491:
URL: https://github.com/apache/pinot/pull/18491#issuecomment-4661192317

   After rebasing on current master, I found that PR #18484 (merged upstream) already added a comprehensive `hasOnlyRetryableQueryExceptions()` helper that handles this case — and more: it covers `BROKER_INSTANCE_MISSING`, `BROKER_SEGMENT_UNAVAILABLE`, and several other retryable codes via the `RETRYABLE_QUERY_ERROR_CODES` set, and the `getErrorCode()` helper safely handles null errorCode nodes (the original NPE trigger).
   
   Our fix is fully superseded. Closing this PR to avoid duplication.


-- 
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] Fix NPE in StreamOp.fetchExistingTotalDocs when exceptions array is present [pinot]

Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia closed pull request #18491: [fix] Fix NPE in StreamOp.fetchExistingTotalDocs when exceptions array is present
URL: https://github.com/apache/pinot/pull/18491


-- 
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] Fix NPE in StreamOp.fetchExistingTotalDocs when exceptions array is present [pinot]

Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on PR #18491:
URL: https://github.com/apache/pinot/pull/18491#issuecomment-4561608978

   Sure
   On Thu, 28 May 2026 at 12:47 AM, NOOB ***@***.***> wrote:
   
   > ***@***.**** commented on this pull request.
   >
   > apologies for late review, can you pull master and update the branch.
   > Looks like conflict maybe the issue has been fixed.
   >
   > —
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/pinot/pull/18491?email_source=notifications&email_token=AVL2AZQFYQK6JDRXN6MHKCT4445LXA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMZXGUZTMMRQHEYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-4375362090>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AVL2AZRDU57SL477VNBAALD4445LXAVCNFSM6AAAAACY4K2JTKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DGNZVGM3DEMBZGA>
   > .
   > You are receiving this because you authored the thread.Message ID:
   > ***@***.***>
   >
   


-- 
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] Fix NPE in StreamOp.fetchExistingTotalDocs when exceptions array is present [pinot]

Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on PR #18491:
URL: https://github.com/apache/pinot/pull/18491#issuecomment-4440604630

   @noob-se7en please review


-- 
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] Fix NPE in StreamOp.fetchExistingTotalDocs when exceptions array is present [pinot]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #18491:
URL: https://github.com/apache/pinot/pull/18491#issuecomment-4440942324

   ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/18491?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.85%. Comparing base ([`bd76ff0`](https://app.codecov.io/gh/apache/pinot/commit/bd76ff0ee27f5554a4021e3c2a7ec769eb02617d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`00c78d4`](https://app.codecov.io/gh/apache/pinot/commit/00c78d4b44652eee03a9f05aa3cbce9720331304?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   > :exclamation:  There is a different number of reports uploaded between BASE (bd76ff0) and HEAD (00c78d4). Click for more details.
   > 
   > <details><summary>HEAD has 16 uploads less than BASE</summary>
   >
   >| Flag | BASE (bd76ff0) | HEAD (00c78d4) |
   >|------|------|------|
   >|java-21|5|1|
   >|unittests|2|1|
   >|temurin|5|1|
   >|unittests2|1|0|
   >|integration|3|0|
   >|integration1|1|0|
   >|integration2|1|0|
   >|custom-integration1|1|0|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #18491      +/-   ##
   ============================================
   - Coverage     63.68%   55.85%   -7.83%     
   + Complexity     1685      816     -869     
   ============================================
     Files          3266     2555     -711     
     Lines        199821   148278   -51543     
     Branches      31022    23949    -7073     
   ============================================
   - Hits         127257    82824   -44433     
   + Misses        62425    58385    -4040     
   + Partials      10139     7069    -3070     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/pinot/pull/18491/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/18491/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/18491/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/18491/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/18491/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/18491/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.85% <ø> (-7.83%)` | :arrow_down: |
   | [temurin](https://app.codecov.io/gh/apache/pinot/pull/18491/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.85% <ø> (-7.83%)` | :arrow_down: |
   | [unittests](https://app.codecov.io/gh/apache/pinot/pull/18491/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.85% <ø> (-7.83%)` | :arrow_down: |
   | [unittests1](https://app.codecov.io/gh/apache/pinot/pull/18491/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.85% <ø> (+0.02%)` | :arrow_up: |
   | [unittests2](https://app.codecov.io/gh/apache/pinot/pull/18491/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/18491?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