1fanwang commented on code in PR #18721:
URL: https://github.com/apache/hudi/pull/18721#discussion_r3927857525


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -3862,6 +3863,23 @@ private void validate() {
       checkArgument(lookbackCommits >= 0,
           String.format("%s must be non-negative, but was %d",
               ROLLING_METADATA_TIMELINE_LOOKBACK_COMMITS.key(), 
lookbackCommits));
+
+      validateEventTimeConfigs();
+    }
+
+    private void validateEventTimeConfigs() {

Review Comment:
   Done in 89bd6c4f.



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -3862,6 +3863,23 @@ private void validate() {
       checkArgument(lookbackCommits >= 0,
           String.format("%s must be non-negative, but was %d",
               ROLLING_METADATA_TIMELINE_LOOKBACK_COMMITS.key(), 
lookbackCommits));
+
+      validateEventTimeConfigs();
+    }
+
+    private void validateEventTimeConfigs() {
+      // Event-time field is configured but watermark tracking is off — the 
field
+      // will be ignored at commit time. Surface a hint so the user can opt in 
via
+      // TRACK_EVENT_TIME_WATERMARK.
+      String eventTimeFieldName = 
writeConfig.getString(HoodiePayloadProps.PAYLOAD_EVENT_TIME_FIELD_PROP_KEY);
+      if (!StringUtils.isNullOrEmpty(eventTimeFieldName)
+          && !writeConfig.getBooleanOrDefault(TRACK_EVENT_TIME_WATERMARK)) {
+        log.warn("{}={} is configured but {}={}; event-time watermark metadata 
will not be tracked. "
+                + "Set {}=true to record event-time watermark in commit 
metadata.",
+            HoodiePayloadProps.PAYLOAD_EVENT_TIME_FIELD_PROP_KEY, 
eventTimeFieldName,
+            TRACK_EVENT_TIME_WATERMARK.key(), 
writeConfig.getBooleanOrDefault(TRACK_EVENT_TIME_WATERMARK),

Review Comment:
   Done in 89bd6c4f.



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -3883,6 +3884,23 @@ private void validate() {
       checkArgument(ttlStatsMaxParallelism > 0,
           String.format("%s must be positive, but was %d",
               HoodieTTLConfig.STATS_MAX_PARALLELISM.key(), 
ttlStatsMaxParallelism));
+
+      validateEventTimeConfigs();
+    }
+
+    private void validateEventTimeConfigs() {
+      // Event-time field is configured but watermark tracking is off — the 
field
+      // will be ignored at commit time. Surface a hint so the user can opt in 
via
+      // TRACK_EVENT_TIME_WATERMARK.
+      String eventTimeFieldName = 
writeConfig.getString(HoodiePayloadProps.PAYLOAD_EVENT_TIME_FIELD_PROP_KEY);
+      if (!StringUtils.isNullOrEmpty(eventTimeFieldName)
+          && !writeConfig.getBooleanOrDefault(TRACK_EVENT_TIME_WATERMARK)) {
+        log.warn("{}={} is configured but {}={}; event-time watermark metadata 
will not be tracked. "
+                + "Set {}=true to record event-time watermark in commit 
metadata.",
+            HoodiePayloadProps.PAYLOAD_EVENT_TIME_FIELD_PROP_KEY, 
eventTimeFieldName,
+            TRACK_EVENT_TIME_WATERMARK.key(), 
writeConfig.getBooleanOrDefault(TRACK_EVENT_TIME_WATERMARK),

Review Comment:
   Done in 89bd6c4f.



##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/config/TestHoodieWriteConfig.java:
##########
@@ -985,4 +993,34 @@ public void 
testUpdatesStrategyNotOverriddenWhenExplicitlySet() {
         .build();
     assertEquals(customStrategy, 
writeConfig.getClusteringUpdatesStrategyClass());
   }
+
+  @Test
+  public void testWarnsWhenEventTimeFieldSetWithoutWatermarkTracking() {
+    Properties props = new Properties();
+    props.setProperty(HoodiePayloadConfig.EVENT_TIME_FIELD.key(), "ts");
+    List<LogEvent> captured = new ArrayList<>();
+    AbstractAppender appender = new AbstractAppender(
+        "CaptureAppender", null, null, true, Property.EMPTY_ARRAY) {
+      @Override
+      public void append(LogEvent event) {
+        captured.add(event.toImmutable());
+      }
+    };
+    appender.start();
+    Logger logger = (Logger) LogManager.getLogger(HoodieWriteConfig.class);
+    logger.addAppender(appender);
+    try {
+      
HoodieWriteConfig.newBuilder().withPath("/tmp").withProperties(props).build();
+    } finally {
+      logger.removeAppender(appender);

Review Comment:
   Done in 90b2fe12.



-- 
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]

Reply via email to