vinothchandar commented on a change in pull request #4035:
URL: https://github.com/apache/hudi/pull/4035#discussion_r756393001
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieMetadataConfig.java
##########
@@ -138,6 +138,12 @@
.sinceVersion("0.10.0")
.withDocumentation("When enabled, populates all meta fields. When
disabled, no meta fields are populated.");
+ public static final ConfigProperty<Boolean>
VALIDATE_METADATA_PAYLOAD_CONSISTENCY = ConfigProperty
+ .key("_" + METADATA_PREFIX + ".validate.metadata.payload.consistency")
+ .defaultValue(false)
+ .sinceVersion("0.10.10")
+ .withDocumentation("Whether to perform validations on metadata payload
consistency.");
Review comment:
this needs to be bit more descriptive
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java
##########
@@ -218,6 +218,8 @@ protected void commit(HoodieTable table, String
commitActionType, String instant
HoodieActiveTimeline activeTimeline = table.getActiveTimeline();
// Finalize write
finalizeWrite(table, instantTime, stats);
+ // update Metadata table
+ writeTableMetadata(table, instantTime, commitActionType, metadata);
Review comment:
so this was missing before?
##########
File path:
hudi-common/src/main/java/org/apache/hudi/metadata/BaseTableMetadata.java
##########
@@ -157,8 +157,13 @@ protected BaseTableMetadata(HoodieEngineContext
engineContext, HoodieMetadataCon
List<String> partitions = Collections.emptyList();
if (hoodieRecord.isPresent()) {
if (!hoodieRecord.get().getData().getDeletions().isEmpty()) {
Review comment:
+1
##########
File path:
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/client/functional/TestHoodieMetadataBase.java
##########
@@ -290,6 +292,7 @@ protected HoodieWriteConfig getWriteConfig(boolean
autoCommit, boolean useFileLi
.enableFullScan(enableFullScan)
.enableMetrics(enableMetrics)
.withPopulateMetaFields(false)
+ .validateMetadataPayloadConsistency(validateMetadataPayloadConsistency)
Review comment:
+1
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieMetadataConfig.java
##########
@@ -138,6 +138,12 @@
.sinceVersion("0.10.0")
.withDocumentation("When enabled, populates all meta fields. When
disabled, no meta fields are populated.");
+ public static final ConfigProperty<Boolean>
VALIDATE_METADATA_PAYLOAD_CONSISTENCY = ConfigProperty
+ .key("_" + METADATA_PREFIX + ".validate.metadata.payload.consistency")
+ .defaultValue(false)
+ .sinceVersion("0.10.10")
+ .withDocumentation("Whether to perform validations on metadata payload
consistency.");
Review comment:
can we can this something like `.ignore.spurious.deletes` which is what
it is. i.e deleting something that did not exist in the first place.
validatexxxx makes it sound like something essential being turned off
##########
File path:
hudi-common/src/main/java/org/apache/hudi/metadata/BaseTableMetadata.java
##########
@@ -191,8 +196,13 @@ protected BaseTableMetadata(HoodieEngineContext
engineContext, HoodieMetadataCon
FileStatus[] statuses = {};
if (hoodieRecord.isPresent()) {
if (!hoodieRecord.get().getData().getDeletions().isEmpty()) {
- throw new HoodieMetadataException("Metadata record for partition " +
partitionName + " is inconsistent: "
- + hoodieRecord.get().getData());
+ if (metadataConfig.validateMetadataPayloadConsistency()) {
Review comment:
avoid code duplication and use methods?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
