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 <[email protected]>
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: [email protected]
For additional commands, e-mail: [email protected]
