siddharthteotia commented on a change in pull request #6869:
URL: https://github.com/apache/incubator-pinot/pull/6869#discussion_r626365949
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/recommender/rules/impl/PinotTablePartitionRule.java
##########
@@ -119,12 +113,15 @@ public void run()
//We define have a desirable segment size OPTIMAL_SIZE_PER_SEGMENT
//Divide the size of data coming in on a given day by
OPTIMAL_SIZE_PER_SEGMENT we get the number of partitions.
if
(!_input.getOverWrittenConfigs().getPartitionConfig().isNumPartitionsOfflineOverwritten())
{
- _output.getPartitionConfig().setNumPartitionsOffline((int)
(optimalOfflinePartitions));
+ int optimalOfflinePartitions = (int)
_output.getSegmentSizeRecommendations().getNumSegments();
+
_output.getPartitionConfig().setNumPartitionsOffline(optimalOfflinePartitions);
}
}
if (_input.getTableType().equalsIgnoreCase(HYBRID)) {
if
(!_input.getOverWrittenConfigs().getPartitionConfig().isNumPartitionsOfflineOverwritten())
{
-
_output.getPartitionConfig().setNumPartitionsOffline(Math.min(optimalOfflinePartitions,
numKafkaPartitions));
+ int optimalOfflinePartitions =
+ Math.min((int)
_output.getSegmentSizeRecommendations().getNumSegments(), numKafkaPartitions);
+
_output.getPartitionConfig().setNumPartitionsOffline(optimalOfflinePartitions);
Review comment:
Do we need to add additional check on cardinality further below where we
pick the column to partition on? I remember discussing the case where
cardinality < numPartitions
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
