Concurrent modification of responseContext in QueryRunners

[leventov]

responseContext is a plain HashMap. It is accessed/modified concurrently without any synchronization in AsyncQueryRunner, IntervalChunkingQueryRunner, ChainedExecutionQueryFactory, etc.

We didn't notice this problem before, because responseContext had very limited use, but as part of #3798 I started to use responseContexts more extensively and got ConcurrentModificationException.

I quickly fixed this in this commit: metamx@54c2af2 but I don't like this solution, especially how it works in AsyncQueryRunner and DirectDruidClient, because it is super fragile and relies on what particular types of QueryRunners call AsyncQueryRunner and called from DirectDruidClient. There are two main problems:

• it is not specified how responseContext could and could not be used and when it is "finalized": after return from QueryRunner.run(), or after iteration over the returned Sequence is completed.
• No difference between "sync" and "async" (lazy) QueryRunners/Sequences

I don't have an idea yet what a better solution would be.

[gianm]

How about making the response context thread-safe?

Currently, the response context is returned as an HTTP header, so it should be "finalized" after QueryRunner.run returns, before iteration over the Sequence. But it might be more useful to allow it to be finalized after iteration. #3319 and #3323 are considering changing the context return mechanism to allow this.

[himanshug]

also, please keep in mind that stuff in responseContext generated on historicals is transferred to brokers (ok, its kinda broken right now due to the header length limits but #3323 would fix that) , so it shouldn't have too much irrelevant stuff that does not need to be sent across.
goal of responseContext is to be able to pass through query execution contextual information to the end-user.

[stale]

This issue has been marked as stale due to 280 days of inactivity. It will be closed in 2 weeks if no further activity occurs. If this issue is still relevant, please simply write any comment. Even if closed, you can still revive the issue at any time or discuss it on the dev@druid.apache.org list. Thank you for your contributions.

[stale]

This issue has been closed due to lack of activity. If you think that is incorrect, or the issue requires additional review, you can revive the issue at any time.