uncoveredIntervals can overflow query response header

[jon-wei]

CachingClusteredClient builds up a list of "uncovered intervals", intervals within the query interval that had no data in the underlying segments.

This list is returned as part of the response header in the "X-Druid-Response-Context" field.

If there are a large number of uncovered intervals, the response header may not be large enough to hold this list. On my local system I saw the header buffer had an 8KB size limit.

I noticed this while running a SegmentMetadataQuery on a set of minute-granularity segments that only contained data for even minutes.

e.g.

2015-12-16T17:33:21,646 WARN [qtp296974277-44] org.eclipse.jetty.server.HttpChannel - /druid/v2/?pretty
java.io.IOException: Response header too large
    at org.eclipse.jetty.http.HttpGenerator.generateResponse(HttpGenerator.java:400) ~[jetty-http-9.2.5.v20141112.jar:9.2.5.v20141112]
    at org.eclipse.jetty.server.HttpConnection$SendCallback.process(HttpConnection.java:599) ~[jetty-server-9.2.5.v20141112.jar:9.2.5.v20141112]
    at org.eclipse.jetty.util.IteratingCallback.processing(IteratingCallback.java:246) ~[jetty-util-9.2.5.v20141112.jar:9.2.5.v20141112]
    at org.eclipse.jetty.util.IteratingCallback.iterate(IteratingCallback.java:208) ~[jetty-util-9.2.5.v20141112.jar:9.2.5.v20141112]
    at org.eclipse.jetty.server.HttpConnection.send(HttpConnection.java:448) ~[jetty-server-9.2.5.v20141112.jar:9.2.5.v20141112]
    at org.eclipse.jetty.server.HttpChannel.sendResponse(HttpChannel.java:762) ~[jetty-server-9.2.5.v20141112.jar:9.2.5.v20141112]
    at org.eclipse.jetty.server.HttpChannel.write(HttpChannel.java:800) ~[jetty-server-9.2.5.v20141112.jar:9.2.5.v20141112]
    at org.eclipse.jetty.server.HttpOutput.write(HttpOutput.java:139) ~[jetty-server-9.2.5.v20141112.jar:9.2.5.v20141112]
    at org.eclipse.jetty.server.HttpOutput.write(HttpOutput.java:132) ~[jetty-server-9.2.5.v20141112.jar:9.2.5.v20141112]
    at org.eclipse.jetty.server.HttpOutput.write(HttpOutput.java:347) ~[jetty-server-9.2.5.v20141112.jar:9.2.5.v20141112]
...
Caused by: java.nio.BufferOverflowException
    at java.nio.Buffer.nextPutIndex(Buffer.java:521) ~[?:1.8.0_51]
    at java.nio.HeapByteBuffer.put(HeapByteBuffer.java:169) ~[?:1.8.0_51]
    at org.eclipse.jetty.http.HttpGenerator.putSanitisedValue(HttpGenerator.java:1057) ~[jetty-http-9.2.5.v20141112.jar:9.2.5.v20141112]
    at org.eclipse.jetty.http.HttpGenerator.putTo(HttpGenerator.java:1079) ~[jetty-http-9.2.5.v20141112.jar:9.2.5.v20141112]
    at org.eclipse.jetty.http.HttpGenerator.generateHeaders(HttpGenerator.java:703) ~[jetty-http-9.2.5.v20141112.jar:9.2.5.v20141112]
    at org.eclipse.jetty.http.HttpGenerator.generateResponse(HttpGenerator.java:385) ~[jetty-http-9.2.5.v20141112.jar:9.2.5.v20141112]
    ... 74 more

[himanshug]

that is worrying.
@cheddar how about only returning a flag telling whether "partial" data is being returned or make this feature configurable based on some query config parameter?

[nishantmonu51]

We can also look into a way of returning metadata thing in the query result itself instead of header.
@cheddar Any thoughts ?

[jaehc]

Is there any update on this?

[xvrl]

@himanshug I think we should make this feature an optional query flag, and disable it by default until there is a more robust implementation.

[himanshug]

@xvrl SGTM ... let me do a PR to make that configurable.

[cheddar]

So, there are a couple of fixes that need to be done I think.

1. The big one that this highlights is that we do not have any size checks on the payload sent back in the header. We should fix that and have something that verifies the size of what we are sending back. If it is too large, we should log and send back a different header instead.

   The fix to add "uncoveredIntervals" has exposed the bug, but it is not the only thing that could potentially cause problems here.

2. I'm fine with hiding uncoveredIntervals behind a feature flag. We could also add a configurable limit to how many intervals are put in the list so that we try to make sure that we aren't overloading things. If we have more than 20 intervals, for example, we just say "many intervals didn't have data" and move on.

[drcrallen]

There should be a configuration in jetty to increase the allowed header size as well. Currently there's not a great way to configure jetty options within Druid though.

[himanshug]

so (2) is still needed and (1) needs to be done in parallel. created #2331 to track (1) separately.

[xvrl]

@drcrallen while there is theoretically no limit to http header sizes, I am worried increasing the header size could cause interoperability issues with browsers, proxies, or other http clients querying Druid. Most of those set limits on the size of http headers, which the user may not have much control over.

[cheddar]

Fwiw, I think that the "solution" Druid implements for (1) can be as simple "if the content of the query ocntext map is greater than 2kb, log it and replace it with a UUID that can be used to find the log"

[himanshug]

@xvrl @cheddar @drcrallen pls put comments relevant to (1) in #2331