clintropolis commented on PR #14184:
URL: https://github.com/apache/druid/pull/14184#issuecomment-1529372418
>Hrm, I think I had expected to see a HavingSpec that threw an exception on
`setQuery` in order to repro the behavior that we saw in the wild, but I'm not
seeing that in the tests. Just to help avoid regressions in the future, I think
it would be good to have a test that throws an exception out of a Having spec
and explicitly tests for what has and has not been run so far before that gets
thrown (specifically, it is thrown before any actual work has been done).
I guess I tested this indirectly with
```
Sequence<?> sequence = scheduler.run(report1, Sequences.empty());
// making the sequence doesn't count, only running it does
Assert.assertEquals(5, scheduler.getTotalAvailableCapacity());
```
which is asserting that the lock is no longer grabbed eagerly so anything
outside of the sequence can freely explode and the right thing will happen. I'm
not sure we have any test setup that has the full set of stuff and an
observable `QueryScheduler` to do this exact test, will look around and see how
much trouble it would be to write a test ensuring that a having filter
exploding when making value matchers can't leak locks on the broker.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
