You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by xb...@apache.org on 2024/06/20 18:29:13 UTC
(pinot) branch master updated: Improve upsert compaction threshold validations (#13424)
This is an automated email from the ASF dual-hosted git repository.
xbli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 6883a4dfa5 Improve upsert compaction threshold validations (#13424)
6883a4dfa5 is described below
commit 6883a4dfa59eccfe0572f9837ee1f354649bb56d
Author: Pratik Tibrewal <ti...@uber.com>
AuthorDate: Thu Jun 20 23:59:07 2024 +0530
Improve upsert compaction threshold validations (#13424)
* Improve upsert compaction threshold validations
---
.../upsertcompaction/UpsertCompactionTaskGenerator.java | 6 +++---
.../apache/pinot/segment/local/utils/TableConfigUtils.java | 6 +++---
.../pinot/segment/local/utils/TableConfigUtilsTest.java | 12 ++++++++++--
3 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGenerator.java b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGenerator.java
index f63836f19b..0357bca6fe 100644
--- a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGenerator.java
+++ b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGenerator.java
@@ -57,7 +57,7 @@ public class UpsertCompactionTaskGenerator extends BaseTaskGenerator {
private static final Logger LOGGER = LoggerFactory.getLogger(UpsertCompactionTaskGenerator.class);
private static final String DEFAULT_BUFFER_PERIOD = "7d";
private static final double DEFAULT_INVALID_RECORDS_THRESHOLD_PERCENT = 0.0;
- private static final long DEFAULT_INVALID_RECORDS_THRESHOLD_COUNT = 0;
+ private static final long DEFAULT_INVALID_RECORDS_THRESHOLD_COUNT = 1;
private static final int DEFAULT_NUM_SEGMENTS_BATCH_PER_SERVER_REQUEST = 500;
public static class SegmentSelectionResult {
@@ -239,8 +239,8 @@ public class UpsertCompactionTaskGenerator extends BaseTaskGenerator {
double invalidRecordPercent = ((double) totalInvalidDocs / totalDocs) * 100;
if (totalInvalidDocs == totalDocs) {
segmentsForDeletion.add(segment.getSegmentName());
- } else if (invalidRecordPercent > invalidRecordsThresholdPercent
- && totalInvalidDocs > invalidRecordsThresholdCount) {
+ } else if (invalidRecordPercent >= invalidRecordsThresholdPercent
+ && totalInvalidDocs >= invalidRecordsThresholdCount) {
segmentsForCompaction.add(Pair.of(segment, totalInvalidDocs));
}
}
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
index 6485c3bf03..94de0afa8b 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
@@ -745,11 +745,11 @@ public final class TableConfigUtils {
if (taskTypeConfig.containsKey("bufferTimePeriod")) {
TimeUtils.convertPeriodToMillis(taskTypeConfig.get("bufferTimePeriod"));
}
- // check maxNumRecordsPerSegment
+ // check invalidRecordsThresholdPercent
if (taskTypeConfig.containsKey("invalidRecordsThresholdPercent")) {
- Preconditions.checkState(Double.parseDouble(taskTypeConfig.get("invalidRecordsThresholdPercent")) > 0
+ Preconditions.checkState(Double.parseDouble(taskTypeConfig.get("invalidRecordsThresholdPercent")) >= 0
&& Double.parseDouble(taskTypeConfig.get("invalidRecordsThresholdPercent")) <= 100,
- "invalidRecordsThresholdPercent must be > 0 and <= 100");
+ "invalidRecordsThresholdPercent must be >= 0 and <= 100");
}
// check invalidRecordsThresholdCount
if (taskTypeConfig.containsKey("invalidRecordsThresholdCount")) {
diff --git a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java
index e1ab636420..8062b422f0 100644
--- a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java
+++ b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java
@@ -2355,14 +2355,22 @@ public class TableConfigUtilsTest {
TableConfigUtils.validateTaskConfigs(tableConfig, schema);
- // test with invalid invalidRecordsThresholdPercents
+ // test with invalidRecordsThresholdPercents as 0
upsertCompactionTaskConfig = ImmutableMap.of("invalidRecordsThresholdPercent", "0");
TableConfig zeroPercentTableConfig = new TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME)
.setUpsertConfig(upsertConfig)
.setTaskConfig(new TableTaskConfig(ImmutableMap.of("UpsertCompactionTask", upsertCompactionTaskConfig)))
.build();
+ TableConfigUtils.validateTaskConfigs(zeroPercentTableConfig, schema);
+
+ // test with invalid invalidRecordsThresholdPercents as -1 and 110
+ upsertCompactionTaskConfig = ImmutableMap.of("invalidRecordsThresholdPercent", "-1");
+ TableConfig negativePercentTableConfig = new TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME)
+ .setUpsertConfig(upsertConfig)
+ .setTaskConfig(new TableTaskConfig(ImmutableMap.of("UpsertCompactionTask", upsertCompactionTaskConfig)))
+ .build();
Assert.assertThrows(IllegalStateException.class,
- () -> TableConfigUtils.validateTaskConfigs(zeroPercentTableConfig, schema));
+ () -> TableConfigUtils.validateTaskConfigs(negativePercentTableConfig, schema));
upsertCompactionTaskConfig = ImmutableMap.of("invalidRecordsThresholdPercent", "110");
TableConfig hundredTenPercentTableConfig = new TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME)
.setUpsertConfig(new UpsertConfig(UpsertConfig.Mode.FULL))
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org