Increase default dynamic filter collection limits #17130
Conversation
raunaqmorarka
force-pushed
the
large-df
branch
9 times, most recently
from
aff6be3 to
bd487e9
Compare
I've added a couple of small tweaks and made the increases smaller based on benchmarks. PTAL again |
| @@ -341,9 +341,12 @@ public boolean isAwaitable() | |||
| @Override | |||
| public TupleDomain<ColumnHandle> getCurrentPredicate() | |||
| { | |||
| Set<DynamicFilterId> completedDynamicFilters = dynamicFilters.stream() | |||
There was a problem hiding this comment.
what's the trick here? Avoiding stream?
There was a problem hiding this comment.
Each invocation to getDynamicFilterSummaries was creating a new map of completed dynamic filters and then we would lookup filter in that map.
I just simplified that to lookup in existing map directly.
core/trino-main/src/main/java/io/trino/execution/DynamicFilterConfig.java
Outdated
Show resolved
Hide resolved
| @@ -418,7 +418,7 @@ private ChannelFilter( | |||
| blockTypeOperators.getEqualOperator(type), | |||
| blockTypeOperators.getHashCodeOperator(type), | |||
| blockBuilder, | |||
| Math.min(maxDistinctValues, 8192) * 2, | |||
There was a problem hiding this comment.
Once we switch to higher default, the sizing factor on maxDistinctValues doesn't need to be as aggressive.
The previous value of 8192 also turned out to be good in microbenchmarking but a bit over eager in using memory for actual queries. Not big enough to notice any change in peak memory usage but I decided to make it more conservative anyway.
Avoid constructing RangeView
Resizing blockbuilder is not as expensive as rehashing or a high fill ratio in TypedSet. So we can be more conservative in it's sizing to stay under size limits
|
Do we need docs for any of these? Or updates to existing docs? |
Some of these properties are mentioned in dynamic filtering docs, we want to avoid advertising these low level knobs as we want users to stick to only using |
Description
Increase default dynamic filter collection limits
Additional context and related issues
Potentially alternative to #16875
Release notes
(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: