Introduced includeTrailerHeader to enable TrailerHeaders in response

[vivek807]

Introduced includeTrailerHeader to enable TrailerHeaders in response
If enabled, a header X-Error-Message will be added to indicate reasons for partial results.

Description

Currently, when requesting large amounts of data, the result may be truncated due to query timeouts or other issues. To address this, we are using a non-standard solution.

    // Terminate the last output line, then write an extra blank line, so users can tell the response was not cut off.
    outputStream.write(new byte[]{'\n', '\n'});

To handle this in a standard way, we can implement a trailer header that indicates partial results and the reason for the truncation.

This PR has:

• been self-reviewed.
  • using the concurrency checklist (Remove this item if the PR doesn't have any relation to concurrency.)
• added documentation for new or modified features or behaviors.
• a release note entry in the PR description.
• added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
• added or updated version, license, or notice information in licenses.yaml
• added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
• added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
• added integration tests.
• been tested in a test Druid cluster.

[abhishekagarwal87]

You may want to look into an old PR of mine (now stale) - #13492 - The solution in the PR stopped working after query resources were refactored and I didn't get the time to dig into the reason. But given you are in this area, maybe you can explore that patch.

[vivek807]

You may want to look into an old PR of mine (now stale) - #13492 - The solution in the PR stopped working after query resources were refactored and I didn't get the time to dig into the reason. But given you are in this area, maybe you can explore that patch.

Thanks @abhishekagarwal87 for your comment.
We added a trailer header to indicate truncated results when fetching large amounts of data. For instance, if a query times out after streaming some results, the count and the actual result may mismatch. To signal this, we included an error message in the trailer header to indicate the reason for truncated results, which can be enabled in query context

[nozjkoitop]

Hi @abhishekagarwal87,

While it's not straightforward to resolve the query result truncation issue, this solution is aimed at highlighting when results are truncated and helping to identify the reason. The rationale is pretty legit - we don't want to truncate the query results silently.

@vivek807, could you please revert the formatting changes in the documentation?

[vivek807]

Hi @abhishekagarwal87,

While it's not straightforward to resolve the query result truncation issue, this solution is aimed at highlighting when results are truncated and helping to identify the reason. The rationale is pretty legit - we don't want to truncate the query results silently.

@vivek807, could you please revert the formatting changes in the documentation?

Updated, thanks @nozjkoitop

[abhishekagarwal87]

thanks. I have some comments and yet to go over the test code. Is the test code based on some existing tests?

[adarshsanjeev]

@vivek807 I was trying out this change. I was able to see this header only if I was directly hitting the broker. Hitting the router (that is, localhost:8888 if you run it locally) does not add the header. Have you seen this behaviors as well?

[vivek807]

@vivek807 I was trying out this change. I was able to see this header only if I was directly hitting the broker. Hitting the router (that is, localhost:8888 if you run it locally) does not add the header. Have you seen this behaviors as well?

@adarshsanjeev, Apologies for the delayed response—I somehow missed your comment. After a quick review, it appears that Jetty’s AsyncProxyServlet does not inherently support forwarding trailers in the HTTP/1.1 or HTTP/2 Trailer field. I’ll investigate further to see if this can be addressed.