Jackie-Jiang commented on code in PR #14662:
URL: https://github.com/apache/pinot/pull/14662#discussion_r1920984093
##########
pinot-core/src/main/java/org/apache/pinot/core/data/table/IndexedTable.java:
##########
@@ -19,24 +19,45 @@
package org.apache.pinot.core.data.table;
import com.google.common.base.Preconditions;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
import org.apache.pinot.common.request.context.ExpressionContext;
import org.apache.pinot.common.utils.DataSchema;
import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
import org.apache.pinot.core.query.aggregation.function.AggregationFunction;
import org.apache.pinot.core.query.request.context.QueryContext;
+import org.apache.pinot.core.util.trace.TraceCallable;
/**
* Base implementation of Map-based Table for indexed lookup
*/
@SuppressWarnings({"rawtypes", "unchecked"})
public abstract class IndexedTable extends BaseTable {
+ private static final int THREAD_POOL_SIZE =
Math.max(Runtime.getRuntime().availableProcessors(), 1);
+ private static final ThreadPoolExecutor EXECUTOR_SERVICE =
(ThreadPoolExecutor) Executors.newFixedThreadPool(
Review Comment:
We should pass in a thread pool, instead of configuring a new thread pool
for this. There is no way to control total thread usage this way
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -434,6 +434,8 @@ public static class QueryOptionKey {
/** Max number of groups GroupByDataTableReducer (running at broker)
should return. */
public static final String MIN_BROKER_GROUP_TRIM_SIZE =
"minBrokerGroupTrimSize";
+ public static final String NUM_THREADS_FOR_FINAL_REDUCE =
"numThreadsForFinalReduce";
Review Comment:
The naming is a little bit confusing. By reading the name, I'd imagine this
is configuring the threads on broker reduce. This is parallelizing the final
result extract for group by. We want to make it more specific
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/ArrayTest.java:
##########
@@ -873,7 +873,7 @@ public Schema createSchema() {
}
@Override
- public File createAvroFile()
+ public List<File> createAvroFiles()
Review Comment:
Are the test changes related?
--
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]
