Pinot returns corresponding http response code for errors #17341

pengding-stripe · 2025-12-09T20:02:42Z

BLUF
This PR addresses #17329, adds support to let Pinot broker return corresponding http response error code when query ends up with exceptions, instead of always returning 200

Problem
Pinot today always returns 200 with exceptions list in query response. Client cannot simply depend on the response code to know if the query succeeds or not. There are some use cases that are not easy to explode the response to check query result. For example, outlier detection implemented at the L7 networking layer. So there are needs to let Pinot http response code reflect the actual query response status

Solution
To make this change backward-compatible, we introduced a request header X-PINOT-HTTP-RESPONSE-CODE-REPRESENT-ERROR to indicate Pinot should return http response code representing actual error instead of 200 when it's set to true. So this won't affect all existing use cases, client can make the decision by configuring the header when sending the request.

We also introduced a mapping of http response code <> pinot error code. This should be update when new error code is added, otherwise default to 500

Test
We tested on our test cluster by sending request to a non-existent table, we got 200 back with Pinot error code 450

curl -X POST 'localhost:8099/query' \   
  -H 'Content-Type: application/json' \
  -H 'Accept: application/json' \
  -d '{
  "sql": "select * from fake_table limit 1",
  "trace": false,
  "queryOptions": ""
  }' \
   -v

...
< HTTP/1.1 200 OK
< X-Pinot-Error-Code: 450
...

Then we send the same request with the header set to true, this time we got 500 back which is the corresponding http response code of 450

curl -X POST 'localhost:8099/query' \
  -H 'Content-Type: application/json' \
  -H 'Accept: application/json' \
  -H 'X-PINOT-HTTP-RESPONSE-CODE-REPRESENT-ERROR: true' \
  -d '{
  "sql": "select * from fake_table limit 1",
  "trace": false,
  "queryOptions": ""
}' \
  -v
...
< HTTP/1.1 500 Internal Server Error
< X-Pinot-Error-Code: 450
...

codecov-commenter · 2025-12-09T20:47:53Z

Codecov Report

❌ Patch coverage is 82.22222% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.32%. Comparing base (d03eda3) to head (d73bf42).
⚠️ Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
...pinot/broker/api/resources/PinotClientRequest.java	55.55%	4 Missing ⚠️
...org/apache/pinot/spi/exception/QueryErrorCode.java	91.42%	3 Missing ⚠️
...ot/broker/api/resources/ResponseStoreResource.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #17341      +/-   ##
============================================
+ Coverage     63.28%   63.32%   +0.04%     
  Complexity     1474     1474              
============================================
  Files          3135     3135              
  Lines        186477   186608     +131     
  Branches      28495    28511      +16     
============================================
+ Hits         118006   118176     +170     
+ Misses        59357    59306      -51     
- Partials       9114     9126      +12

Flag	Coverage Δ
custom-integration1	`100.00% <ø> (ø)`
integration	`100.00% <ø> (ø)`
integration1	`100.00% <ø> (ø)`
integration2	`0.00% <ø> (ø)`
java-11	`63.27% <82.22%> (+<0.01%)`	⬆️
java-21	`63.25% <82.22%> (+0.03%)`	⬆️
temurin	`63.32% <82.22%> (+0.04%)`	⬆️
unittests	`63.32% <82.22%> (+0.04%)`	⬆️
unittests1	`55.66% <91.42%> (-0.01%)`	⬇️
unittests2	`34.00% <82.22%> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Jackie-Jiang

Mostly good. Well done

Jackie-Jiang · 2025-12-09T21:33:46Z

pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java

        IN_PROGRESS, // The segment is still consuming data
        COMMITTING, // This state will only be utilised by pauseless ingestion when the segment has been consumed but
-                    // is yet to be build and uploaded by the server.
+        // is yet to be build and uploaded by the server.


(nit) Revert

Jackie-Jiang · 2025-12-09T21:36:44Z

pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java

    public static final String SEGMENT_NAME_HTTP_HEADER = "Pinot-Segment-Name";
    public static final String TABLE_NAME_HTTP_HEADER = "Pinot-Table-Name";
    public static final String PINOT_QUERY_ERROR_CODE_HEADER = "X-Pinot-Error-Code";
+    public static final String PINOT_HTTP_RESPONSE_CODE_REPRESENT_ERROR_HEADER =


This header is for broker request, not controller.
Seems the recent standard deprecated the X- prefix, so I'd suggest naming it Pinot-Use-Http-Status-For-Errors

Jackie-Jiang · 2025-12-09T21:40:52Z

pinot-spi/pom.xml

      <artifactId>assertj-core</artifactId>
      <scope>test</scope>
    </dependency>
+      <dependency>


Move this above (probably above plexus-classworlds) and fix the indentation

Jackie-Jiang · 2025-12-09T21:43:52Z

pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java

      queryErrorCodeHeaderValue = exceptions.get(0).getErrorCode();

+      // Check if the client wants actual HTTP error codes instead of 200 OK
+      if ("true".equalsIgnoreCase(httpHeaders.getHeaderString(


(minor) Use Boolean.parseBoolean()

pengding-stripe · 2025-12-09T23:40:11Z

@Jackie-Jiang addressed feedback, thanks

Jackie-Jiang

LGTM with minor comments

Jackie-Jiang · 2025-12-09T23:50:17Z

pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java

        "pinot.broker.use.mse.to.fill.empty.response.schema";
    public static final boolean DEFAULT_USE_MSE_TO_FILL_EMPTY_RESPONSE_SCHEMA = false;
+
+    public static final String PINOT_USE_HTTP_STATUS_FOR_ERRORS_HEADER =


Suggested change

public static final String PINOT_USE_HTTP_STATUS_FOR_ERRORS_HEADER =

public static final String USE_HTTP_STATUS_FOR_ERRORS_HEADER =

Jackie-Jiang · 2025-12-09T23:50:46Z

pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java

   *
-   * @param brokerResponse
+   * By default, returns HTTP 200 OK even for errors. If the request header
+   * 'X-PINOT-HTTP-RESPONSE-CODE-REPRESENT-ERROR' is set to 'true', returns appropriate HTTP status


Update this

pengding-stripe · 2025-12-10T00:10:08Z

@Jackie-Jiang updated, can you merge this after build pass?

pengding-stripe added 4 commits December 9, 2025 10:56

Extend QueryErrorCode to include corresponding http response code

4e98bf5

return http response code if request header is set to true

95c589e

format

a3d9d54

update response code map

eea4f5b

pengding-stripe changed the title ~~Extend QueryErrorCode to include corresponding http response code~~ Pinot returns corresponding http response code for errors Dec 9, 2025

fix test

4ee352e

Jackie-Jiang reviewed Dec 9, 2025

View reviewed changes

Jackie-Jiang added feature rest-api labels Dec 9, 2025

feedback

3c33d86

Jackie-Jiang approved these changes Dec 9, 2025

View reviewed changes

update naming

d73bf42

Jackie-Jiang merged commit 1abdbf5 into apache:master Dec 10, 2025
20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Pinot returns corresponding http response code for errors #17341

Pinot returns corresponding http response code for errors #17341

Uh oh!

pengding-stripe commented Dec 9, 2025 •

edited

Loading

Uh oh!

codecov-commenter commented Dec 9, 2025 •

edited

Loading

Uh oh!

Jackie-Jiang left a comment

Uh oh!

Jackie-Jiang Dec 9, 2025

Uh oh!

Jackie-Jiang Dec 9, 2025

Uh oh!

Jackie-Jiang Dec 9, 2025

Uh oh!

Jackie-Jiang Dec 9, 2025

Uh oh!

pengding-stripe commented Dec 9, 2025

Uh oh!

Jackie-Jiang left a comment

Uh oh!

Jackie-Jiang Dec 9, 2025

Uh oh!

Jackie-Jiang Dec 9, 2025

Uh oh!

pengding-stripe commented Dec 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	public static final String PINOT_USE_HTTP_STATUS_FOR_ERRORS_HEADER =
	public static final String USE_HTTP_STATUS_FOR_ERRORS_HEADER =

Pinot returns corresponding http response code for errors #17341

Pinot returns corresponding http response code for errors #17341

Uh oh!

Conversation

pengding-stripe commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Jackie-Jiang left a comment

Choose a reason for hiding this comment

Uh oh!

Jackie-Jiang Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Jackie-Jiang Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Jackie-Jiang Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Jackie-Jiang Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

pengding-stripe commented Dec 9, 2025

Uh oh!

Jackie-Jiang left a comment

Choose a reason for hiding this comment

Uh oh!

Jackie-Jiang Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Jackie-Jiang Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

pengding-stripe commented Dec 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pengding-stripe commented Dec 9, 2025 •

edited

Loading

codecov-commenter commented Dec 9, 2025 •

edited

Loading