morrySnow commented on code in PR #48757: URL: https://github.com/apache/doris/pull/48757#discussion_r1984562275
########## fe/fe-core/src/main/java/org/apache/doris/statistics/OlapAnalysisTask.java: ########## @@ -190,8 +199,16 @@ protected Pair<List<Long>, Long> getSampleTablets() { for (int i = 0; i < tabletCounts; i++) { int seekTid = (int) ((i + seek) % ids.size()); long tabletId = ids.get(seekTid); - sampleTabletIds.add(tabletId); long tabletRows = materializedIndex.getTablet(tabletId).getMinReplicaRowCount(p.getVisibleVersion()); + if (tabletRows > MAXIMUM_SAMPLE_ROWS) { + // Skip very large tablet. + largeTabletId = tabletId; Review Comment: we should record the smallest one in the large tablet list ########## fe/fe-core/src/main/java/org/apache/doris/statistics/OlapAnalysisTask.java: ########## @@ -372,12 +396,64 @@ protected String getIndex() { } } - protected long pickSamplePartition(List<Partition> partitions, List<Long> pickedTabletIds) { - long averageRowsPerPartition = tbl.getRowCount() / partitions.size(); + // For partition tables with single time type partition column, we'd better to skip sampling the partition + // that contains all the history data. Because this partition may contain many old data which is not + // visited by most queries. To sample this partition may cause the statistics not accurate. + // For example, one table has 366 partitions, partition 1 ~ 365 store date for each day of the year from now. + // Partition 0 stores all the history data earlier than 1 year. We want to skip sampling partition 0. + protected long getSkipPartitionId(List<Partition> partitions) { + if (partitions == null || partitions.size() < StatisticsUtil.getPartitionSampleCount()) { + return NO_SKIP_TABLET_ID; + } + PartitionInfo partitionInfo = ((OlapTable) tbl).getPartitionInfo(); + if (!PartitionType.RANGE.equals(partitionInfo.getType())) { + return NO_SKIP_TABLET_ID; + } + if (partitionInfo.getPartitionColumns().size() != 1) { + return NO_SKIP_TABLET_ID; + } + Column column = partitionInfo.getPartitionColumns().get(0); + if (!column.getType().isDateType()) { + return NO_SKIP_TABLET_ID; + } + PartitionKey lowestKey = PartitionKey.createMaxPartitionKey(); + long lowestPartitionId = -1; + for (Partition p : partitions) { + RangePartitionItem item = (RangePartitionItem) partitionInfo.getItem(p.getId()); + Range<PartitionKey> items = item.getItems(); + if (!items.hasLowerBound()) { + lowestPartitionId = p.getId(); + break; + } + if (items.lowerEndpoint().compareTo(lowestKey) < 0) { + lowestKey = items.lowerEndpoint(); + lowestPartitionId = p.getId(); + } + } + return lowestPartitionId; + } + + protected long pickSamplePartition(List<Partition> partitions, List<Long> pickedTabletIds, long skipPartition) { + Partition partition = ((OlapTable) tbl).getPartition(skipPartition); + long averageRowsPerPartition; + if (partition != null) { + LOG.info("Going to skip partition {} in table {}", skipPartition, tbl.getName()); + // If we want to skip the oldest partition, calculate the average rows per partition value without + // the oldest partition, otherwise if the oldest partition is very large, we may skip all partitions. + // Because we only pick partitions partitionRowCount >= averageRowsPerPartition. + Preconditions.checkState(partitions.size() > 1); + averageRowsPerPartition = (tbl.getRowCount() - partition.getRowCount()) / (partitions.size() - 1); + } else { + averageRowsPerPartition = tbl.getRowCount() / partitions.size(); + } long indexId = info.indexId == -1 ? ((OlapTable) tbl).getBaseIndexId() : info.indexId; long pickedRows = 0; int pickedPartitionCount = 0; for (Partition p : partitions) { + if (skipPartition == p.getId()) { + LOG.info("Partition {} in table {} skipped", skipPartition, tbl.getName()); Review Comment: debug? ########## fe/fe-core/src/main/java/org/apache/doris/statistics/OlapAnalysisTask.java: ########## @@ -372,12 +396,64 @@ protected String getIndex() { } } - protected long pickSamplePartition(List<Partition> partitions, List<Long> pickedTabletIds) { - long averageRowsPerPartition = tbl.getRowCount() / partitions.size(); + // For partition tables with single time type partition column, we'd better to skip sampling the partition + // that contains all the history data. Because this partition may contain many old data which is not + // visited by most queries. To sample this partition may cause the statistics not accurate. + // For example, one table has 366 partitions, partition 1 ~ 365 store date for each day of the year from now. + // Partition 0 stores all the history data earlier than 1 year. We want to skip sampling partition 0. + protected long getSkipPartitionId(List<Partition> partitions) { + if (partitions == null || partitions.size() < StatisticsUtil.getPartitionSampleCount()) { + return NO_SKIP_TABLET_ID; + } + PartitionInfo partitionInfo = ((OlapTable) tbl).getPartitionInfo(); + if (!PartitionType.RANGE.equals(partitionInfo.getType())) { + return NO_SKIP_TABLET_ID; + } + if (partitionInfo.getPartitionColumns().size() != 1) { + return NO_SKIP_TABLET_ID; + } + Column column = partitionInfo.getPartitionColumns().get(0); + if (!column.getType().isDateType()) { + return NO_SKIP_TABLET_ID; + } + PartitionKey lowestKey = PartitionKey.createMaxPartitionKey(); + long lowestPartitionId = -1; + for (Partition p : partitions) { + RangePartitionItem item = (RangePartitionItem) partitionInfo.getItem(p.getId()); + Range<PartitionKey> items = item.getItems(); + if (!items.hasLowerBound()) { + lowestPartitionId = p.getId(); + break; + } + if (items.lowerEndpoint().compareTo(lowestKey) < 0) { + lowestKey = items.lowerEndpoint(); + lowestPartitionId = p.getId(); + } + } + return lowestPartitionId; + } + + protected long pickSamplePartition(List<Partition> partitions, List<Long> pickedTabletIds, long skipPartition) { + Partition partition = ((OlapTable) tbl).getPartition(skipPartition); + long averageRowsPerPartition; + if (partition != null) { + LOG.info("Going to skip partition {} in table {}", skipPartition, tbl.getName()); Review Comment: log debug is ok? ########## fe/fe-core/src/main/java/org/apache/doris/statistics/OlapAnalysisTask.java: ########## @@ -372,12 +396,64 @@ protected String getIndex() { } } - protected long pickSamplePartition(List<Partition> partitions, List<Long> pickedTabletIds) { - long averageRowsPerPartition = tbl.getRowCount() / partitions.size(); + // For partition tables with single time type partition column, we'd better to skip sampling the partition + // that contains all the history data. Because this partition may contain many old data which is not + // visited by most queries. To sample this partition may cause the statistics not accurate. + // For example, one table has 366 partitions, partition 1 ~ 365 store date for each day of the year from now. + // Partition 0 stores all the history data earlier than 1 year. We want to skip sampling partition 0. + protected long getSkipPartitionId(List<Partition> partitions) { + if (partitions == null || partitions.size() < StatisticsUtil.getPartitionSampleCount()) { + return NO_SKIP_TABLET_ID; + } + PartitionInfo partitionInfo = ((OlapTable) tbl).getPartitionInfo(); + if (!PartitionType.RANGE.equals(partitionInfo.getType())) { + return NO_SKIP_TABLET_ID; + } + if (partitionInfo.getPartitionColumns().size() != 1) { + return NO_SKIP_TABLET_ID; + } + Column column = partitionInfo.getPartitionColumns().get(0); + if (!column.getType().isDateType()) { + return NO_SKIP_TABLET_ID; + } + PartitionKey lowestKey = PartitionKey.createMaxPartitionKey(); + long lowestPartitionId = -1; + for (Partition p : partitions) { + RangePartitionItem item = (RangePartitionItem) partitionInfo.getItem(p.getId()); + Range<PartitionKey> items = item.getItems(); + if (!items.hasLowerBound()) { + lowestPartitionId = p.getId(); + break; + } + if (items.lowerEndpoint().compareTo(lowestKey) < 0) { + lowestKey = items.lowerEndpoint(); + lowestPartitionId = p.getId(); + } + } + return lowestPartitionId; + } + + protected long pickSamplePartition(List<Partition> partitions, List<Long> pickedTabletIds, long skipPartition) { Review Comment: ```suggestion protected long pickSamplePartition(List<Partition> partitions, List<Long> pickedTabletIds, long skipPartitionId) { ``` ########## fe/fe-core/src/main/java/org/apache/doris/statistics/OlapAnalysisTask.java: ########## @@ -372,12 +396,64 @@ protected String getIndex() { } } - protected long pickSamplePartition(List<Partition> partitions, List<Long> pickedTabletIds) { - long averageRowsPerPartition = tbl.getRowCount() / partitions.size(); + // For partition tables with single time type partition column, we'd better to skip sampling the partition + // that contains all the history data. Because this partition may contain many old data which is not + // visited by most queries. To sample this partition may cause the statistics not accurate. + // For example, one table has 366 partitions, partition 1 ~ 365 store date for each day of the year from now. + // Partition 0 stores all the history data earlier than 1 year. We want to skip sampling partition 0. + protected long getSkipPartitionId(List<Partition> partitions) { + if (partitions == null || partitions.size() < StatisticsUtil.getPartitionSampleCount()) { + return NO_SKIP_TABLET_ID; + } + PartitionInfo partitionInfo = ((OlapTable) tbl).getPartitionInfo(); + if (!PartitionType.RANGE.equals(partitionInfo.getType())) { + return NO_SKIP_TABLET_ID; + } + if (partitionInfo.getPartitionColumns().size() != 1) { + return NO_SKIP_TABLET_ID; + } + Column column = partitionInfo.getPartitionColumns().get(0); + if (!column.getType().isDateType()) { + return NO_SKIP_TABLET_ID; + } + PartitionKey lowestKey = PartitionKey.createMaxPartitionKey(); + long lowestPartitionId = -1; + for (Partition p : partitions) { + RangePartitionItem item = (RangePartitionItem) partitionInfo.getItem(p.getId()); + Range<PartitionKey> items = item.getItems(); + if (!items.hasLowerBound()) { + lowestPartitionId = p.getId(); + break; + } + if (items.lowerEndpoint().compareTo(lowestKey) < 0) { + lowestKey = items.lowerEndpoint(); + lowestPartitionId = p.getId(); + } + } + return lowestPartitionId; + } + + protected long pickSamplePartition(List<Partition> partitions, List<Long> pickedTabletIds, long skipPartition) { + Partition partition = ((OlapTable) tbl).getPartition(skipPartition); + long averageRowsPerPartition; + if (partition != null) { + LOG.info("Going to skip partition {} in table {}", skipPartition, tbl.getName()); + // If we want to skip the oldest partition, calculate the average rows per partition value without + // the oldest partition, otherwise if the oldest partition is very large, we may skip all partitions. + // Because we only pick partitions partitionRowCount >= averageRowsPerPartition. + Preconditions.checkState(partitions.size() > 1); Review Comment: how to ensure partition.size() always greater than 1? ########## fe/fe-core/src/main/java/org/apache/doris/statistics/OlapAnalysisTask.java: ########## @@ -372,12 +396,64 @@ protected String getIndex() { } } - protected long pickSamplePartition(List<Partition> partitions, List<Long> pickedTabletIds) { - long averageRowsPerPartition = tbl.getRowCount() / partitions.size(); + // For partition tables with single time type partition column, we'd better to skip sampling the partition + // that contains all the history data. Because this partition may contain many old data which is not + // visited by most queries. To sample this partition may cause the statistics not accurate. + // For example, one table has 366 partitions, partition 1 ~ 365 store date for each day of the year from now. + // Partition 0 stores all the history data earlier than 1 year. We want to skip sampling partition 0. + protected long getSkipPartitionId(List<Partition> partitions) { + if (partitions == null || partitions.size() < StatisticsUtil.getPartitionSampleCount()) { + return NO_SKIP_TABLET_ID; + } + PartitionInfo partitionInfo = ((OlapTable) tbl).getPartitionInfo(); + if (!PartitionType.RANGE.equals(partitionInfo.getType())) { + return NO_SKIP_TABLET_ID; + } + if (partitionInfo.getPartitionColumns().size() != 1) { + return NO_SKIP_TABLET_ID; + } + Column column = partitionInfo.getPartitionColumns().get(0); + if (!column.getType().isDateType()) { + return NO_SKIP_TABLET_ID; + } + PartitionKey lowestKey = PartitionKey.createMaxPartitionKey(); + long lowestPartitionId = -1; + for (Partition p : partitions) { + RangePartitionItem item = (RangePartitionItem) partitionInfo.getItem(p.getId()); + Range<PartitionKey> items = item.getItems(); + if (!items.hasLowerBound()) { + lowestPartitionId = p.getId(); + break; + } + if (items.lowerEndpoint().compareTo(lowestKey) < 0) { + lowestKey = items.lowerEndpoint(); + lowestPartitionId = p.getId(); + } + } + return lowestPartitionId; + } + + protected long pickSamplePartition(List<Partition> partitions, List<Long> pickedTabletIds, long skipPartition) { + Partition partition = ((OlapTable) tbl).getPartition(skipPartition); + long averageRowsPerPartition; + if (partition != null) { + LOG.info("Going to skip partition {} in table {}", skipPartition, tbl.getName()); + // If we want to skip the oldest partition, calculate the average rows per partition value without + // the oldest partition, otherwise if the oldest partition is very large, we may skip all partitions. + // Because we only pick partitions partitionRowCount >= averageRowsPerPartition. + Preconditions.checkState(partitions.size() > 1); Review Comment: add check message to avoid get a NPE exception -- 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: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org