Skip to content

Commit 57771a1

Browse files
xiangfu0claude
andcommitted
Partitioned group-by combine with per-partition thread-local tables
Replace the default GroupByCombineOperator (shared ConcurrentHashMap) with PartitionedGroupByCombineOperator that uses thread-local per-partition IndexedTables, eliminating cross-thread contention. Key changes: - Thread-local partition tables published once under partition locks - Parallel tree-reduction of partition tables in the final merge - Cached Key.hashCode() at construction (O(1) thereafter) - Object2ObjectOpenHashMap replaces HashMap in SimpleIndexedTable - IndexedTable.merge() reuses existing Keys from source entry set - mergePartitionTable() propagates trim stats from absorbed tables - numGroupByPartitions query option controls partitioning - Sorted combine path preserved for safe-trim with small limits Benchmark (JMH, 32 segments, 8 threads, GROUP BY ORDER BY LIMIT 500): 10K records/segment: 500 groups: 127.6ms -> 2.8ms (45.6x faster) 5K groups: 132.4ms -> 5.9ms (22.4x faster) 50K groups: 144.0ms -> 23.0ms (6.3x faster) 500K groups: 184.8ms -> 76.9ms (2.4x faster) 100K records/segment (high cardinality): 5M groups: 1,934ms -> 637ms (3.0x faster) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 32d061e commit 57771a1

15 files changed

Lines changed: 886 additions & 89 deletions

File tree

pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,12 @@ public static Integer getMinInitialIndexedTableCapacity(Map<String, String> quer
409409
return checkedParseIntPositive(QueryOptionKey.MIN_INITIAL_INDEXED_TABLE_CAPACITY, minInitialIndexedTableCapacity);
410410
}
411411

412+
@Nullable
413+
public static Integer getNumGroupByPartitions(Map<String, String> queryOptions) {
414+
String numGroupByPartitionsString = queryOptions.get(QueryOptionKey.NUM_GROUP_BY_PARTITIONS);
415+
return checkedParseIntPositive(QueryOptionKey.NUM_GROUP_BY_PARTITIONS, numGroupByPartitionsString);
416+
}
417+
412418
public static boolean shouldDropResults(Map<String, String> queryOptions) {
413419
return Boolean.parseBoolean(queryOptions.get(CommonConstants.Broker.Request.QueryOptionKey.DROP_RESULTS));
414420
}

pinot-core/src/main/java/org/apache/pinot/core/data/table/IndexedTable.java

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,23 @@ public boolean upsert(Record record) {
109109
return upsert(new Key(keyValues), record);
110110
}
111111

112+
@Override
113+
public boolean merge(Table table) {
114+
if (table instanceof IndexedTable) {
115+
// Optimized path: iterate the source's entry set directly to reuse existing Key objects,
116+
// avoiding per-record Arrays.copyOf + new Key + Arrays.hashCode overhead.
117+
for (Map.Entry<Key, Record> entry : ((IndexedTable) table)._lookupMap.entrySet()) {
118+
upsert(entry.getKey(), entry.getValue());
119+
}
120+
} else {
121+
Iterator<Record> iterator = table.iterator();
122+
while (iterator.hasNext()) {
123+
upsert(iterator.next());
124+
}
125+
}
126+
return true;
127+
}
128+
112129
/**
113130
* Adds a record with new key or updates a record with existing key.
114131
*/
@@ -263,9 +280,49 @@ public int size() {
263280

264281
@Override
265282
public Iterator<Record> iterator() {
283+
if (_topRecords == null) {
284+
return _lookupMap.values().iterator();
285+
}
266286
return _topRecords.iterator();
267287
}
268288

289+
290+
/**
291+
* Merges records from a disjoint partition table into this table using {@code putAll}.
292+
* <p><b>Precondition:</b> the source table must have no overlapping keys with this table
293+
* (i.e. they come from disjoint hash partitions). If keys overlap, {@code putAll} will
294+
* silently overwrite existing entries instead of aggregating them. Use {@link #merge(Table)}
295+
* for tables that may share keys.
296+
*/
297+
public void mergePartitionTable(Table table) {
298+
_topRecords = null;
299+
if (table instanceof IndexedTable) {
300+
IndexedTable other = (IndexedTable) table;
301+
_lookupMap.putAll(other._lookupMap);
302+
_numResizes += other._numResizes;
303+
_resizeTimeNs += other._resizeTimeNs;
304+
} else {
305+
Iterator<Record> iterator = table.iterator();
306+
while (iterator.hasNext()) {
307+
// NOTE: For performance concern, does not check the return value of the upsert(). Override this method if
308+
// upsert() can return false.
309+
upsert(iterator.next());
310+
}
311+
}
312+
if (_lookupMap.size() >= _trimThreshold) {
313+
resize();
314+
}
315+
}
316+
317+
/**
318+
* Absorbs trim/resize statistics from another table that was merged into this one.
319+
* Call after {@link #merge(Table)} to preserve the absorbed table's trim history.
320+
*/
321+
public void absorbResizeStats(IndexedTable other) {
322+
_numResizes += other._numResizes;
323+
_resizeTimeNs += other._resizeTimeNs;
324+
}
325+
269326
public int getNumResizes() {
270327
return _numResizes;
271328
}

pinot-core/src/main/java/org/apache/pinot/core/data/table/Key.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,11 @@
3838
*/
3939
public class Key implements Comparable<Key> {
4040
private final Object[] _values;
41+
private final int _hashCode;
4142

4243
public Key(Object[] values) {
4344
_values = values;
45+
_hashCode = Arrays.hashCode(values);
4446
}
4547

4648
public Object[] getValues() {
@@ -56,7 +58,7 @@ public boolean equals(Object o) {
5658

5759
@Override
5860
public int hashCode() {
59-
return Arrays.hashCode(_values);
61+
return _hashCode;
6062
}
6163

6264
@Override

pinot-core/src/main/java/org/apache/pinot/core/data/table/SimpleIndexedTable.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,26 @@
1818
*/
1919
package org.apache.pinot.core.data.table;
2020

21-
import java.util.HashMap;
21+
import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap;
2222
import java.util.concurrent.ExecutorService;
2323
import javax.annotation.concurrent.NotThreadSafe;
2424
import org.apache.pinot.common.utils.DataSchema;
2525
import org.apache.pinot.core.query.request.context.QueryContext;
2626

2727

2828
/**
29-
* {@link Table} implementation for aggregating TableRecords based on combination of keys
29+
* {@link Table} implementation for aggregating TableRecords based on combination of keys.
30+
* Uses fastutil's {@link Object2ObjectOpenHashMap} for open-addressing, which provides better
31+
* cache locality and avoids per-entry {@code Map.Entry} object allocations compared to
32+
* {@code java.util.HashMap}.
3033
*/
3134
@NotThreadSafe
3235
public class SimpleIndexedTable extends IndexedTable {
3336

3437
public SimpleIndexedTable(DataSchema dataSchema, boolean hasFinalInput, QueryContext queryContext, int resultSize,
3538
int trimSize, int trimThreshold, int initialCapacity, ExecutorService executorService) {
36-
super(dataSchema, hasFinalInput, queryContext, resultSize, trimSize, trimThreshold, new HashMap<>(initialCapacity),
37-
executorService);
39+
super(dataSchema, hasFinalInput, queryContext, resultSize, trimSize, trimThreshold,
40+
new Object2ObjectOpenHashMap<>(initialCapacity), executorService);
3841
}
3942

4043
/**

pinot-core/src/main/java/org/apache/pinot/core/operator/combine/GroupByCombineOperator.java

Lines changed: 86 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -56,17 +56,17 @@ public class GroupByCombineOperator extends BaseSingleBlockCombineOperator<Group
5656
private static final Logger LOGGER = LoggerFactory.getLogger(GroupByCombineOperator.class);
5757
private static final String EXPLAIN_NAME = "COMBINE_GROUP_BY";
5858

59-
private final int _numAggregationFunctions;
60-
private final int _numGroupByExpressions;
61-
private final int _numColumns;
59+
protected final int _numAggregationFunctions;
60+
protected final int _numGroupByExpressions;
61+
protected final int _numColumns;
6262
// We use a CountDownLatch to track if all Futures are finished by the query timeout, and cancel the unfinished
6363
// _futures (try to interrupt the execution if it already started).
64-
private final CountDownLatch _operatorLatch;
64+
protected final CountDownLatch _operatorLatch;
6565

66-
private volatile IndexedTable _indexedTable;
67-
private volatile boolean _groupsTrimmed;
68-
private volatile boolean _numGroupsLimitReached;
69-
private volatile boolean _numGroupsWarningLimitReached;
66+
protected volatile IndexedTable _indexedTable;
67+
protected volatile boolean _groupsTrimmed;
68+
protected volatile boolean _numGroupsLimitReached;
69+
protected volatile boolean _numGroupsWarningLimitReached;
7070

7171
public GroupByCombineOperator(List<Operator> operators, QueryContext queryContext, ExecutorService executorService) {
7272
super(null, operators, overrideMaxExecutionThreads(queryContext, operators.size()), executorService);
@@ -113,59 +113,11 @@ protected void processSegments() {
113113
if (_indexedTable == null) {
114114
synchronized (this) {
115115
if (_indexedTable == null) {
116-
_indexedTable = GroupByUtils.createIndexedTableForCombineOperator(resultsBlock, _queryContext, _numTasks,
117-
_executorService);
116+
_indexedTable = createIndexedTable(resultsBlock, _numTasks);
118117
}
119118
}
120119
}
121-
122-
if (resultsBlock.isGroupsTrimmed()) {
123-
_groupsTrimmed = true;
124-
}
125-
// Set groups limit reached flag.
126-
if (resultsBlock.isNumGroupsLimitReached()) {
127-
_numGroupsLimitReached = true;
128-
}
129-
if (resultsBlock.isNumGroupsWarningLimitReached()) {
130-
_numGroupsWarningLimitReached = true;
131-
}
132-
133-
// Merge aggregation group-by result.
134-
// Iterate over the group-by keys, for each key, update the group-by result in the indexedTable
135-
Collection<IntermediateRecord> intermediateRecords = resultsBlock.getIntermediateRecords();
136-
// Count the number of merged keys
137-
int mergedKeys = 0;
138-
// For now, only GroupBy OrderBy query has pre-constructed intermediate records
139-
if (intermediateRecords == null) {
140-
// Merge aggregation group-by result.
141-
AggregationGroupByResult aggregationGroupByResult = resultsBlock.getAggregationGroupByResult();
142-
if (aggregationGroupByResult != null) {
143-
// Iterate over the group-by keys, for each key, update the group-by result in the indexedTable
144-
try {
145-
Iterator<GroupKeyGenerator.GroupKey> dicGroupKeyIterator = aggregationGroupByResult.getGroupKeyIterator();
146-
while (dicGroupKeyIterator.hasNext()) {
147-
QueryThreadContext.checkTerminationAndSampleUsagePeriodically(mergedKeys++, EXPLAIN_NAME);
148-
GroupKeyGenerator.GroupKey groupKey = dicGroupKeyIterator.next();
149-
Object[] keys = groupKey._keys;
150-
Object[] values = Arrays.copyOf(keys, _numColumns);
151-
int groupId = groupKey._groupId;
152-
for (int i = 0; i < _numAggregationFunctions; i++) {
153-
values[_numGroupByExpressions + i] = aggregationGroupByResult.getResultForGroupId(i, groupId);
154-
}
155-
_indexedTable.upsert(new Key(keys), new Record(values));
156-
}
157-
} finally {
158-
// Release the resources used by the group key generator
159-
aggregationGroupByResult.closeGroupKeyGenerator();
160-
}
161-
}
162-
} else {
163-
for (IntermediateRecord intermediateResult : intermediateRecords) {
164-
QueryThreadContext.checkTerminationAndSampleUsagePeriodically(mergedKeys++, EXPLAIN_NAME);
165-
//TODO: change upsert api so that it accepts intermediateRecord directly
166-
_indexedTable.upsert(intermediateResult._key, intermediateResult._record);
167-
}
168-
}
120+
mergeGroupByResultsBlock(_indexedTable, resultsBlock, EXPLAIN_NAME);
169121
} catch (RuntimeException e) {
170122
throw wrapOperatorException(operator, e);
171123
} finally {
@@ -176,6 +128,56 @@ protected void processSegments() {
176128
}
177129
}
178130

131+
protected IndexedTable createIndexedTable(GroupByResultsBlock resultsBlock, int numThreads) {
132+
return GroupByUtils.createIndexedTableForCombineOperator(resultsBlock, _queryContext, numThreads, _executorService);
133+
}
134+
135+
protected void updateCombineResultsStats(GroupByResultsBlock resultsBlock) {
136+
if (resultsBlock.isGroupsTrimmed()) {
137+
_groupsTrimmed = true;
138+
}
139+
if (resultsBlock.isNumGroupsLimitReached()) {
140+
_numGroupsLimitReached = true;
141+
}
142+
if (resultsBlock.isNumGroupsWarningLimitReached()) {
143+
_numGroupsWarningLimitReached = true;
144+
}
145+
}
146+
147+
protected void mergeGroupByResultsBlock(IndexedTable indexedTable, GroupByResultsBlock resultsBlock,
148+
String explainName) {
149+
updateCombineResultsStats(resultsBlock);
150+
151+
Collection<IntermediateRecord> intermediateRecords = resultsBlock.getIntermediateRecords();
152+
int mergedKeys = 0;
153+
if (intermediateRecords == null) {
154+
AggregationGroupByResult aggregationGroupByResult = resultsBlock.getAggregationGroupByResult();
155+
if (aggregationGroupByResult != null) {
156+
try {
157+
Iterator<GroupKeyGenerator.GroupKey> dicGroupKeyIterator = aggregationGroupByResult.getGroupKeyIterator();
158+
while (dicGroupKeyIterator.hasNext()) {
159+
QueryThreadContext.checkTerminationAndSampleUsagePeriodically(mergedKeys++, explainName);
160+
GroupKeyGenerator.GroupKey groupKey = dicGroupKeyIterator.next();
161+
Object[] keys = groupKey._keys;
162+
Object[] values = Arrays.copyOf(keys, _numColumns);
163+
int groupId = groupKey._groupId;
164+
for (int i = 0; i < _numAggregationFunctions; i++) {
165+
values[_numGroupByExpressions + i] = aggregationGroupByResult.getResultForGroupId(i, groupId);
166+
}
167+
indexedTable.upsert(new Key(keys), new Record(values));
168+
}
169+
} finally {
170+
aggregationGroupByResult.closeGroupKeyGenerator();
171+
}
172+
}
173+
} else {
174+
for (IntermediateRecord intermediateResult : intermediateRecords) {
175+
QueryThreadContext.checkTerminationAndSampleUsagePeriodically(mergedKeys++, explainName);
176+
indexedTable.upsert(intermediateResult._key, intermediateResult._record);
177+
}
178+
}
179+
}
180+
179181
@Override
180182
public void onProcessSegmentsException(Throwable t) {
181183
_processingException.compareAndSet(null, t);
@@ -205,33 +207,41 @@ public BaseResultsBlock mergeResults()
205207
long timeoutMs = _queryContext.getEndTimeMs() - System.currentTimeMillis();
206208
boolean opCompleted = _operatorLatch.await(timeoutMs, TimeUnit.MILLISECONDS);
207209
if (!opCompleted) {
208-
// If this happens, the broker side should already timed out, just log the error and return
209-
String userError = "Timed out while combining group-by order-by results after " + timeoutMs + "ms";
210-
String logMsg = userError + ", queryContext = " + _queryContext;
211-
LOGGER.error(logMsg);
212-
return new ExceptionResultsBlock(new QueryErrorMessage(QueryErrorCode.EXECUTION_TIMEOUT, userError, logMsg));
210+
return getTimeoutResultsBlock(timeoutMs);
213211
}
214212

215213
Throwable ex = _processingException.get();
216214
if (ex != null) {
217-
String userError = "Caught exception while processing group-by order-by query";
218-
String devError = userError + ": " + ex.getMessage();
219-
QueryErrorMessage errMsg;
220-
if (ex instanceof QueryException) {
221-
// If the exception is a QueryException, use the error code from the exception and trust the error message
222-
errMsg = new QueryErrorMessage(((QueryException) ex).getErrorCode(), devError, devError);
223-
} else {
224-
// If the exception is not a QueryException, use the generic error code and don't expose the exception message
225-
errMsg = new QueryErrorMessage(QueryErrorCode.QUERY_EXECUTION, userError, devError);
226-
}
227-
return new ExceptionResultsBlock(errMsg);
215+
return getExceptionResultsBlock(ex);
228216
}
229217

230-
if (_indexedTable.isTrimmed() && _queryContext.isUnsafeTrim()) {
218+
return getMergedResultsBlock(_indexedTable);
219+
}
220+
221+
protected ExceptionResultsBlock getTimeoutResultsBlock(long timeoutMs) {
222+
String userError = "Timed out while combining group-by results after " + Math.max(0, timeoutMs) + "ms";
223+
String logMsg = userError + ", queryContext = " + _queryContext;
224+
LOGGER.error(logMsg);
225+
return new ExceptionResultsBlock(new QueryErrorMessage(QueryErrorCode.EXECUTION_TIMEOUT, userError, logMsg));
226+
}
227+
228+
protected ExceptionResultsBlock getExceptionResultsBlock(Throwable ex) {
229+
String userError = "Caught exception while processing group-by query";
230+
String devError = userError + ": " + ex.getMessage();
231+
QueryErrorMessage errMsg;
232+
if (ex instanceof QueryException) {
233+
errMsg = new QueryErrorMessage(((QueryException) ex).getErrorCode(), devError, devError);
234+
} else {
235+
errMsg = new QueryErrorMessage(QueryErrorCode.QUERY_EXECUTION, userError, devError);
236+
}
237+
return new ExceptionResultsBlock(errMsg);
238+
}
239+
240+
protected GroupByResultsBlock getMergedResultsBlock(IndexedTable indexedTable) {
241+
if (indexedTable.isTrimmed() && _queryContext.isUnsafeTrim()) {
231242
_groupsTrimmed = true;
232243
}
233244

234-
IndexedTable indexedTable = _indexedTable;
235245
if (_queryContext.isServerReturnFinalResult()) {
236246
indexedTable.finish(true, true);
237247
} else if (_queryContext.isServerReturnFinalResultKeyUnpartitioned()) {

0 commit comments

Comments
 (0)