klsince commented on code in PR #13347:
URL: https://github.com/apache/pinot/pull/13347#discussion_r1717544662
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java:
##########
@@ -858,6 +858,31 @@ static void validateUpsertAndDedupConfig(TableConfig
tableConfig, Schema schema)
}
}
+
+ if (upsertConfig != null &&
upsertConfig.isEnableDeletedKeysCompactionConsistency()) {
+ // enableConsistentDeletes shouldn't exist with metadataTTL
+ Preconditions.checkState(upsertConfig.getMetadataTTL() == 0,
+ "enableConsistentDeletes and metadataTTL shouldn't exist together
for upsert table");
+
+ // enableConsistentDeletes shouldn't exist with enablePreload
+ Preconditions.checkState(!upsertConfig.isEnablePreload(),
+ "enableConsistentDeletes and enablePreload shouldn't exist together
for upsert table");
+
+ // enableConsistentDeletes should exist with deletedKeysTTL
+ Preconditions.checkState(upsertConfig.getDeletedKeysTTL() > 0,
+ "enableConsistentDeletes should exist with deletedKeysTTL for upsert
table");
Review Comment:
update `enableConsistentDeletes` to `enableDeletedKeysCompactionConsistency`
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/TableUpsertMetadataManagerFactory.java:
##########
@@ -29,16 +29,15 @@
public class TableUpsertMetadataManagerFactory {
- private TableUpsertMetadataManagerFactory() {
- }
-
- private static final Logger LOGGER =
LoggerFactory.getLogger(TableUpsertMetadataManagerFactory.class);
public static final String UPSERT_DEFAULT_METADATA_MANAGER_CLASS =
"default.metadata.manager.class";
public static final String UPSERT_DEFAULT_ENABLE_SNAPSHOT =
"default.enable.snapshot";
public static final String UPSERT_DEFAULT_ENABLE_PRELOAD =
"default.enable.preload";
-
public static final String
UPSERT_DEFAULT_ALLOW_PARTIAL_UPSERT_CONSUMPTION_DURING_COMMIT =
"default.allow.partial.upsert.consumption.during.commit";
+ private static final Logger LOGGER =
LoggerFactory.getLogger(TableUpsertMetadataManagerFactory.class);
Review Comment:
looks like we don't need to change this class now that the flag is checked
inside getOrCreatePartitionManager()?
--
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...@pinot.apache.org
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
