-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PROPOSAL] Query vectorization #7093
Comments
I finally found some time to read and review the proposal. I think the overall approach makes a lot of sense, and generally agree with the stages. I think though, that we should consider enabling vectorized queries right away, instead of turning them off. They are a clear win and it will help them actually be validated and used, and force us to test more to ensure they are production ready. |
Thanks for taking a look @fjy. I would prefer to leave vectorization off by default for the first couple of releases at least, since the level of battle-testedness would be a lot lower vs. the 'classic' code paths. But I do plan to enable it at the Druid installations that I run, and at as many others as I could convince to. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Please keep this open. |
Closed by #6794. |
Motivation
Vectorized execution (operating on batches of rows at a time, instead of individual rows) is widely recognized as being crucial to maximizing performance of analytical databases like Druid. It allows queries to be sped up by reducing the number of method calls, allowing more cache-efficiency, and potentially enabling CPU SIMD instructions. (At least, in theory it makes the latter possible, but I haven't looked into whether such instructions are actually being generated in this patch.)
However, Druid today does not have a vectorized query engine. Benchmarks on the implementation in #6794 indicate we could get a 1.3-3x speedup on various kinds of queries. This would represent a major change for how Druid query engines work, but one that would be beneficial, and a long time coming (see #3011 for some earlier discussion).
Proposed changes
The proposal is to enable the following:
In all cases, cutting down on method calls and improving locality are expected benefits.
In the interests of getting a limited, but still useful, set of functionality out at first, I am proposing doing a subset of the work as "Phase 1". The following subsections detail the plan.
New vectorized methods
The following interfaces do not need vectorized analogs, but will instead have vectorized methods added.
canVectorize
,makeVectorCursor
canVectorizeMatcher
,makeVectorMatcher
canVectorize
,decorate(SingleValueDimensionVectorSelector selector)
,decorate(MultiValueDimensionVectorSelector selector)
canVectorize
,factorizeVector
New vectorized interfaces
The following interfaces will have vectorized analogs added. None of the vectorized implementations are annotated with
@HotLoopCallee
, because it is anticipated that monomorphic optimization will not be necessary due to amortization of costs over vectors of rows.getColumnCapabilities
is guaranteed to return non-null capabilities if the column exists. This makes life substantially easier for callers and is doable because, in practice, we don't expecte vectorizable storage adapters to not know what their columns are.Vectorized query engines
The following query engines will have vectorized analogs added. In both cases, vectorization only applies to per-segment processing. Merging is still row-at-a-time.
Timeseries: See TimeseriesQueryEngine's
processVectorized
method.GroupBy: See VectorGroupByEngine. As you can see in
canVectorize
, it doesn't handle multi-value dimensions or granularity other than "all" yet.New context parameters
The following query parameters are added.
false
false
(disabled),true
(enabled if possible, disabled otherwise, on a per-segment basis), andforce
(enabled, and groupBy or timeseries queries that cannot be vectorized will fail). The"force"
setting is meant to aid in testing, and is not generally useful in production (since real-time segments can never be processed with vectorized execution, any queries on real-time data will fail).512
Initial implementation
Initial implementation of "phase 1" is present on #6794. It has the following limitations. I believe the patch is mergeable without these limitations being addressed, however, since it gets the foundation in place and allows more than one person to work on improving it.
Rationale
For the long term health of the Druid project, I don't think there is any alternative to implementing vectorization somehow. Some alternative implementations that I didn't go with include:
Replacing existing classes with vectorized implementations. I thought this would be too disruptive and too big of a potential hit to stability, in the case of bugs found in the vectorized implementations. The biggest issue with this approach is it is a lot of new code to maintain, and it adds a new code path to every dimension spec, filter, aggregator, and query engine. Both non-vectorized and vectorized implementations could be maintained for some time. This may be a quite a while, depending on how long it takes us to feel that vectorization is stable. But for our collective sanity, we may want to consider working on projects that allow us to remove the non-vectorized code paths. This would include vectorizing other query engines (notably topN), adding vectorization support for virtual columns and descending order (neither of which are supported in this patch), and adding vectorized implementations for all aggregators, filters, and dimension specs that don't currently have them.
Vectorizing the entire query processing pipeline. This patch only addresses the per-segment processing, and does not affect how results are merged between segments. There are opportunities for improvement here, especially with topN and groupBy. However, for most common Druid use cases, most of the work is in the per-segment processing stages, because those tend to reduce data sizes substantially (due to aggregation).
Using Apache Arrow to represent and/or process the vectors. I felt this would be too much of a departure from the 'classic' Druid code paths. It would also represent a radical departure from the culture of implementing our column serdes and query processors 'in-house' to allow ourselves maximum flexibility. However, this does not preclude potentially using Arrow as a result format for external responses, or as an inter-process data transfer format, in the future.
Operational impact
The functionality is completely optional, so there is no operational impact if the feature is not used. Even if the feature is used, there is no expected impact to cluster updates, because the inter-process request and response formats have not changed beyond the addition of the "vectorize" context parameter (which is just a hint, anyway, and can be ignored). Heap memory use may increase somewhat if the feature is used, due to the need for query processing operators to allocate arrays for their outputs.
Test plan
Currently, the testing strategy includes adding vectorization-enabled tests to TimeseriesQueryRunnerTest, GroupByQueryRunnerTest, GroupByTimeseriesQueryRunnerTest, CalciteQueryTest, and all of the filtering tests that extend BaseFilterTest. In all of those classes, there are some test cases that don't support vectorization. They are marked by special function calls like "cannotVectorize" or "skipVectorize" that tell the test harness to either expect an exception or to skip the test case.
Testing should be expanded in the future -- a project in and of itself.
Future work
One thing that is not a part of this proposal is vectorization of later parts of the query processing pipeline: anything beyond the per-segment engines. So result merging code on both historicals and brokers would not be modified. It would make sense to do this at some point, but I haven't thought much about how to tackle it.
Future work should also include addressing all of the limitations discussed above in the "Initial implementation" section:
I think with a framework in place like the one this patch adds, it would be possible for more than one person to contribute to the 'future work' stuff.
The text was updated successfully, but these errors were encountered: