You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "lokeshj1703 (via GitHub)" <gi...@apache.org> on 2023/05/04 10:03:46 UTC

[GitHub] [hudi] lokeshj1703 opened a new pull request, #8631: HUDI-6170. Use correct zone id while calculating earliestTimeToRetain

lokeshj1703 opened a new pull request, #8631:
URL: https://github.com/apache/hudi/pull/8631

   ### Change Logs
   
   Currently we use default zoneId while calculating earliestTimeToRetain. Jira aims to use the configured timezone.
   
   ### Impact
   
   NA
   
   ### Risk level (write none, low medium or high below)
   
   low
   
   ### Documentation Update
   
   NA
   
   ### Contributor's checklist
   
   - [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [ ] Change Logs and Impact were stated clearly
   - [ ] Adequate tests were added if applicable
   - [ ] CI passed
   


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] hudi-bot commented on pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8631:
URL: https://github.com/apache/hudi/pull/8631#issuecomment-1538745345

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827",
       "triggerID" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16936",
       "triggerID" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "triggerType" : "PUSH"
     }, {
       "hash" : "5e2f6a913cfb525f4f4e8a92c5009635a642478e",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16942",
       "triggerID" : "5e2f6a913cfb525f4f4e8a92c5009635a642478e",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 5e2f6a913cfb525f4f4e8a92c5009635a642478e Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16942) 
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] hudi-bot commented on pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8631:
URL: https://github.com/apache/hudi/pull/8631#issuecomment-1540481209

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827",
       "triggerID" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16936",
       "triggerID" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "triggerType" : "PUSH"
     }, {
       "hash" : "5e2f6a913cfb525f4f4e8a92c5009635a642478e",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16942",
       "triggerID" : "5e2f6a913cfb525f4f4e8a92c5009635a642478e",
       "triggerType" : "PUSH"
     }, {
       "hash" : "1652e6ee6435b454dcbee662f7deaf7fb45883bf",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "1652e6ee6435b454dcbee662f7deaf7fb45883bf",
       "triggerType" : "PUSH"
     }, {
       "hash" : "b6d499683bf2de6478cf888a76c7e9057a49ac36",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16971",
       "triggerID" : "b6d499683bf2de6478cf888a76c7e9057a49ac36",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 1652e6ee6435b454dcbee662f7deaf7fb45883bf UNKNOWN
   * b6d499683bf2de6478cf888a76c7e9057a49ac36 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16971) 
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] danny0405 commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1185653080


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/HoodieTimelineArchiver.java:
##########
@@ -491,7 +491,7 @@ private Stream<HoodieInstant> getCommitInstantsToArchive() throws IOException {
         String latestCommitToArchive = instantsToArchive.get(instantsToArchive.size() - 1).getTimestamp();
         try {
           Instant latestCommitInstant = HoodieActiveTimeline.parseDateFromInstantTime(commitTimeline.lastInstant().get().getTimestamp()).toInstant();
-          ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(latestCommitInstant, ZoneId.systemDefault());
+          ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(latestCommitInstant, HoodieInstantTimeGenerator.getTimelineTimeZone().getZoneId());
           String earliestTimeToRetain = HoodieActiveTimeline.formatDate(Date.from(currentDateTime.minusHours(config.getCleanerHoursRetained()).toInstant()));

Review Comment:
   The code only works when `HoodieInstantTimeGenerator.setCommitTimeZone` is executed in the same JVM process, we must asure that. Either set it up explicitly or instantiate a `HoodieTableConfig` within which the timezone is set up.



##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanPlanner.java:
##########
@@ -510,7 +510,7 @@ public Option<HoodieInstant> getEarliestCommitToRetain() {
       }
     } else if (config.getCleanerPolicy() == HoodieCleaningPolicy.KEEP_LATEST_BY_HOURS) {
       Instant instant = Instant.now();
-      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, ZoneId.systemDefault());
+      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, HoodieInstantTimeGenerator.getTimelineTimeZone().getZoneId());
       String earliestTimeToRetain = HoodieActiveTimeline.formatDate(Date.from(currentDateTime.minusHours(hoursRetained).toInstant()));

Review Comment:
   The code only works when `HoodieInstantTimeGenerator.setCommitTimeZone` is executed in the same JVM process, we must asure that. Either set it up explicitly or instantiate a `HoodieTableConfig` within which the timezone is set up.



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] hudi-bot commented on pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8631:
URL: https://github.com/apache/hudi/pull/8631#issuecomment-1534928316

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827",
       "triggerID" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * ba5b45adead50081d48c057447e7a4854a952c7d Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827) 
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] danny0405 commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1189298111


##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieTimelineTimeZone.java:
##########
@@ -18,20 +18,29 @@
 
 package org.apache.hudi.common.model;
 
+import java.time.ZoneId;
+import java.util.TimeZone;
+
 /**
  * Hoodie TimelineZone.
  */
 public enum HoodieTimelineTimeZone {
-  LOCAL("local"),
-  UTC("utc");
+  LOCAL("local", ZoneId.systemDefault()),
+  UTC("utc", TimeZone.getTimeZone("UTC").toZoneId());
 
   private final String timeZone;
+  private final ZoneId zoneId;

Review Comment:
   Fine, it's just a preference.



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] lokeshj1703 commented on pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "lokeshj1703 (via GitHub)" <gi...@apache.org>.
lokeshj1703 commented on PR #8631:
URL: https://github.com/apache/hudi/pull/8631#issuecomment-1538228674

   I see some failures in Azure CI, where option value is not present for timeline timezone in `HoodieInstantTimeGenerator`.


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] nsivabalan commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1185719699


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanPlanner.java:
##########
@@ -510,7 +510,7 @@ public Option<HoodieInstant> getEarliestCommitToRetain() {
       }
     } else if (config.getCleanerPolicy() == HoodieCleaningPolicy.KEEP_LATEST_BY_HOURS) {
       Instant instant = Instant.now();
-      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, ZoneId.systemDefault());
+      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, HoodieInstantTimeGenerator.getTimelineTimeZone().getZoneId());
       String earliestTimeToRetain = HoodieActiveTimeline.formatDate(Date.from(currentDateTime.minusHours(hoursRetained).toInstant()));

Review Comment:
   tableConfig is generated for the first time when table is created and the time zone is set right then. after that, we can't alter the time zone. 
   This is in line w/ how we generate our commit times as well. 
   
   not sure if we need any fixes here. can you help clarify



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] lokeshj1703 commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "lokeshj1703 (via GitHub)" <gi...@apache.org>.
lokeshj1703 commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1188376891


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanPlanner.java:
##########
@@ -510,7 +510,7 @@ public Option<HoodieInstant> getEarliestCommitToRetain() {
       }
     } else if (config.getCleanerPolicy() == HoodieCleaningPolicy.KEEP_LATEST_BY_HOURS) {
       Instant instant = Instant.now();
-      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, ZoneId.systemDefault());
+      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, HoodieInstantTimeGenerator.getTimelineTimeZone().getZoneId());
       String earliestTimeToRetain = HoodieActiveTimeline.formatDate(Date.from(currentDateTime.minusHours(hoursRetained).toInstant()));

Review Comment:
   Yeah. We can handle it in a separate jira though.



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] hudi-bot commented on pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8631:
URL: https://github.com/apache/hudi/pull/8631#issuecomment-1538200420

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827",
       "triggerID" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16936",
       "triggerID" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 91d8e920a47668f0f1bde5dc1258ec2fcb349181 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16936) 
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] hudi-bot commented on pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8631:
URL: https://github.com/apache/hudi/pull/8631#issuecomment-1538271445

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827",
       "triggerID" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16936",
       "triggerID" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "triggerType" : "PUSH"
     }, {
       "hash" : "5e2f6a913cfb525f4f4e8a92c5009635a642478e",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "5e2f6a913cfb525f4f4e8a92c5009635a642478e",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 91d8e920a47668f0f1bde5dc1258ec2fcb349181 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16936) 
   * 5e2f6a913cfb525f4f4e8a92c5009635a642478e UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] nsivabalan commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1187089107


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanPlanner.java:
##########
@@ -510,7 +510,7 @@ public Option<HoodieInstant> getEarliestCommitToRetain() {
       }
     } else if (config.getCleanerPolicy() == HoodieCleaningPolicy.KEEP_LATEST_BY_HOURS) {
       Instant instant = Instant.now();
-      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, ZoneId.systemDefault());
+      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, HoodieInstantTimeGenerator.getTimelineTimeZone().getZoneId());
       String earliestTimeToRetain = HoodieActiveTimeline.formatDate(Date.from(currentDateTime.minusHours(hoursRetained).toInstant()));

Review Comment:
   I see the issue. may be we should not set any default value for commitTimeZone in HoodieInstantTimeGenerator. we can set it to null infact. And so unless the table properties are instantiation whcih in turn will call into setCommitTimeZone, if any other callers tries to access the commit time zone, will fail fast. 
   



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] lokeshj1703 commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "lokeshj1703 (via GitHub)" <gi...@apache.org>.
lokeshj1703 commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1188232721


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanPlanner.java:
##########
@@ -510,7 +510,7 @@ public Option<HoodieInstant> getEarliestCommitToRetain() {
       }
     } else if (config.getCleanerPolicy() == HoodieCleaningPolicy.KEEP_LATEST_BY_HOURS) {
       Instant instant = Instant.now();
-      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, ZoneId.systemDefault());
+      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, HoodieInstantTimeGenerator.getTimelineTimeZone().getZoneId());
       String earliestTimeToRetain = HoodieActiveTimeline.formatDate(Date.from(currentDateTime.minusHours(hoursRetained).toInstant()));

Review Comment:
   I have changed the PR to use metaClient's tableConfig. Please take a look.
   I think `HoodieInstantTimeGenerator` usage needs some refactoring. The static access doesn't seem right.



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] hudi-bot commented on pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8631:
URL: https://github.com/apache/hudi/pull/8631#issuecomment-1534550882

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827",
       "triggerID" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * ba5b45adead50081d48c057447e7a4854a952c7d Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827) 
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] hudi-bot commented on pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8631:
URL: https://github.com/apache/hudi/pull/8631#issuecomment-1539873262

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827",
       "triggerID" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16936",
       "triggerID" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "triggerType" : "PUSH"
     }, {
       "hash" : "5e2f6a913cfb525f4f4e8a92c5009635a642478e",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16942",
       "triggerID" : "5e2f6a913cfb525f4f4e8a92c5009635a642478e",
       "triggerType" : "PUSH"
     }, {
       "hash" : "1652e6ee6435b454dcbee662f7deaf7fb45883bf",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "1652e6ee6435b454dcbee662f7deaf7fb45883bf",
       "triggerType" : "PUSH"
     }, {
       "hash" : "b6d499683bf2de6478cf888a76c7e9057a49ac36",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16971",
       "triggerID" : "b6d499683bf2de6478cf888a76c7e9057a49ac36",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 5e2f6a913cfb525f4f4e8a92c5009635a642478e Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16942) 
   * 1652e6ee6435b454dcbee662f7deaf7fb45883bf UNKNOWN
   * b6d499683bf2de6478cf888a76c7e9057a49ac36 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16971) 
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] lokeshj1703 commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "lokeshj1703 (via GitHub)" <gi...@apache.org>.
lokeshj1703 commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1188380042


##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieInstantTimeGenerator.java:
##########
@@ -135,7 +133,7 @@ private static TemporalAccessor convertDateToTemporalAccessor(Date d) {
   }
 
   public static void setCommitTimeZone(HoodieTimelineTimeZone commitTimeZone) {
-    commitTimeZoneOpt = Option.of(commitTimeZone);
+    HoodieInstantTimeGenerator.commitTimeZone = commitTimeZone;

Review Comment:
   Do you mean making this method synchronized?



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] danny0405 commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1185651903


##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieTimelineTimeZone.java:
##########
@@ -18,20 +18,29 @@
 
 package org.apache.hudi.common.model;
 
+import java.time.ZoneId;
+import java.util.TimeZone;
+
 /**
  * Hoodie TimelineZone.
  */
 public enum HoodieTimelineTimeZone {
-  LOCAL("local"),
-  UTC("utc");
+  LOCAL("local", ZoneId.systemDefault()),
+  UTC("utc", TimeZone.getTimeZone("UTC").toZoneId());
 
   private final String timeZone;
+  private final ZoneId zoneId;

Review Comment:
   Don't think we need a zoneId member, can we calculate it on the fly in method `getZoneId` ?



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] danny0405 commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1185813955


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanPlanner.java:
##########
@@ -510,7 +510,7 @@ public Option<HoodieInstant> getEarliestCommitToRetain() {
       }
     } else if (config.getCleanerPolicy() == HoodieCleaningPolicy.KEEP_LATEST_BY_HOURS) {
       Instant instant = Instant.now();
-      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, ZoneId.systemDefault());
+      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, HoodieInstantTimeGenerator.getTimelineTimeZone().getZoneId());
       String earliestTimeToRetain = HoodieActiveTimeline.formatDate(Date.from(currentDateTime.minusHours(hoursRetained).toInstant()));

Review Comment:
   > tableConfig is generated for the first time when table is created
   
   Can we always ensure table config is initialized first? How to guard this sequence dependency.



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] danny0405 commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1188264786


##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieInstantTimeGenerator.java:
##########
@@ -135,7 +133,7 @@ private static TemporalAccessor convertDateToTemporalAccessor(Date d) {
   }
 
   public static void setCommitTimeZone(HoodieTimelineTimeZone commitTimeZone) {
-    commitTimeZoneOpt = Option.of(commitTimeZone);
+    HoodieInstantTimeGenerator.commitTimeZone = commitTimeZone;

Review Comment:
   Should make it singleton by using a `synchromized` lock



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] hudi-bot commented on pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8631:
URL: https://github.com/apache/hudi/pull/8631#issuecomment-1539601957

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827",
       "triggerID" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16936",
       "triggerID" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "triggerType" : "PUSH"
     }, {
       "hash" : "5e2f6a913cfb525f4f4e8a92c5009635a642478e",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16942",
       "triggerID" : "5e2f6a913cfb525f4f4e8a92c5009635a642478e",
       "triggerType" : "PUSH"
     }, {
       "hash" : "1652e6ee6435b454dcbee662f7deaf7fb45883bf",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "1652e6ee6435b454dcbee662f7deaf7fb45883bf",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 5e2f6a913cfb525f4f4e8a92c5009635a642478e Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16942) 
   * 1652e6ee6435b454dcbee662f7deaf7fb45883bf UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] danny0405 merged pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 merged PR #8631:
URL: https://github.com/apache/hudi/pull/8631


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] hudi-bot commented on pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8631:
URL: https://github.com/apache/hudi/pull/8631#issuecomment-1537965449

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827",
       "triggerID" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16936",
       "triggerID" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * ba5b45adead50081d48c057447e7a4854a952c7d Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827) 
   * 91d8e920a47668f0f1bde5dc1258ec2fcb349181 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16936) 
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] lokeshj1703 commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "lokeshj1703 (via GitHub)" <gi...@apache.org>.
lokeshj1703 commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1187365382


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanPlanner.java:
##########
@@ -510,7 +510,7 @@ public Option<HoodieInstant> getEarliestCommitToRetain() {
       }
     } else if (config.getCleanerPolicy() == HoodieCleaningPolicy.KEEP_LATEST_BY_HOURS) {
       Instant instant = Instant.now();
-      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, ZoneId.systemDefault());
+      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, HoodieInstantTimeGenerator.getTimelineTimeZone().getZoneId());
       String earliestTimeToRetain = HoodieActiveTimeline.formatDate(Date.from(currentDateTime.minusHours(hoursRetained).toInstant()));

Review Comment:
   I made it an option in the latest commit but I see some failures in Azure CI where option value is not present for timeline timezone. Its in hudi-cli and `HoodieJavaGenerateApp`, both of these use `HoodieInstantTimeGenerator` directly without setting timezone.



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] hudi-bot commented on pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8631:
URL: https://github.com/apache/hudi/pull/8631#issuecomment-1534471842

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * ba5b45adead50081d48c057447e7a4854a952c7d UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] lokeshj1703 commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "lokeshj1703 (via GitHub)" <gi...@apache.org>.
lokeshj1703 commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1188380042


##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieInstantTimeGenerator.java:
##########
@@ -135,7 +133,7 @@ private static TemporalAccessor convertDateToTemporalAccessor(Date d) {
   }
 
   public static void setCommitTimeZone(HoodieTimelineTimeZone commitTimeZone) {
-    commitTimeZoneOpt = Option.of(commitTimeZone);
+    HoodieInstantTimeGenerator.commitTimeZone = commitTimeZone;

Review Comment:
   Do you mean making this method synchronized or making `HoodieInstantTimeGenerator` a singleton?



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] hudi-bot commented on pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8631:
URL: https://github.com/apache/hudi/pull/8631#issuecomment-1537953508

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827",
       "triggerID" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * ba5b45adead50081d48c057447e7a4854a952c7d Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827) 
   * 91d8e920a47668f0f1bde5dc1258ec2fcb349181 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] danny0405 commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1188063597


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanPlanner.java:
##########
@@ -510,7 +510,7 @@ public Option<HoodieInstant> getEarliestCommitToRetain() {
       }
     } else if (config.getCleanerPolicy() == HoodieCleaningPolicy.KEEP_LATEST_BY_HOURS) {
       Instant instant = Instant.now();
-      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, ZoneId.systemDefault());
+      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, HoodieInstantTimeGenerator.getTimelineTimeZone().getZoneId());
       String earliestTimeToRetain = HoodieActiveTimeline.formatDate(Date.from(currentDateTime.minusHours(hoursRetained).toInstant()));

Review Comment:
   Yeah, that means the code is prune to making misusages, let's fix all those test falures by initialzing the zoneId manually.



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] lokeshj1703 commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "lokeshj1703 (via GitHub)" <gi...@apache.org>.
lokeshj1703 commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1185732010


##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieTimelineTimeZone.java:
##########
@@ -18,20 +18,29 @@
 
 package org.apache.hudi.common.model;
 
+import java.time.ZoneId;
+import java.util.TimeZone;
+
 /**
  * Hoodie TimelineZone.
  */
 public enum HoodieTimelineTimeZone {
-  LOCAL("local"),
-  UTC("utc");
+  LOCAL("local", ZoneId.systemDefault()),
+  UTC("utc", TimeZone.getTimeZone("UTC").toZoneId());
 
   private final String timeZone;
+  private final ZoneId zoneId;

Review Comment:
   I can do a switch-case but I feel this is better. WDYT?



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] danny0405 commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1188261901


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanPlanner.java:
##########
@@ -510,7 +510,7 @@ public Option<HoodieInstant> getEarliestCommitToRetain() {
       }
     } else if (config.getCleanerPolicy() == HoodieCleaningPolicy.KEEP_LATEST_BY_HOURS) {
       Instant instant = Instant.now();
-      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, ZoneId.systemDefault());
+      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, HoodieInstantTimeGenerator.getTimelineTimeZone().getZoneId());
       String earliestTimeToRetain = HoodieActiveTimeline.formatDate(Date.from(currentDateTime.minusHours(hoursRetained).toInstant()));

Review Comment:
   > I think HoodieInstantTimeGenerator usage needs some refactoring
   
   Yes, maybe we should pass in the table path as param and triggers a lazy initialization for the zoneId if necessary.



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] hudi-bot commented on pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8631:
URL: https://github.com/apache/hudi/pull/8631#issuecomment-1539789547

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827",
       "triggerID" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16936",
       "triggerID" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "triggerType" : "PUSH"
     }, {
       "hash" : "5e2f6a913cfb525f4f4e8a92c5009635a642478e",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16942",
       "triggerID" : "5e2f6a913cfb525f4f4e8a92c5009635a642478e",
       "triggerType" : "PUSH"
     }, {
       "hash" : "1652e6ee6435b454dcbee662f7deaf7fb45883bf",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "1652e6ee6435b454dcbee662f7deaf7fb45883bf",
       "triggerType" : "PUSH"
     }, {
       "hash" : "b6d499683bf2de6478cf888a76c7e9057a49ac36",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b6d499683bf2de6478cf888a76c7e9057a49ac36",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 5e2f6a913cfb525f4f4e8a92c5009635a642478e Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16942) 
   * 1652e6ee6435b454dcbee662f7deaf7fb45883bf UNKNOWN
   * b6d499683bf2de6478cf888a76c7e9057a49ac36 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] hudi-bot commented on pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8631:
URL: https://github.com/apache/hudi/pull/8631#issuecomment-1538281389

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16827",
       "triggerID" : "ba5b45adead50081d48c057447e7a4854a952c7d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16936",
       "triggerID" : "91d8e920a47668f0f1bde5dc1258ec2fcb349181",
       "triggerType" : "PUSH"
     }, {
       "hash" : "5e2f6a913cfb525f4f4e8a92c5009635a642478e",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16942",
       "triggerID" : "5e2f6a913cfb525f4f4e8a92c5009635a642478e",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 91d8e920a47668f0f1bde5dc1258ec2fcb349181 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16936) 
   * 5e2f6a913cfb525f4f4e8a92c5009635a642478e Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16942) 
   
   <details>
   <summary>Bot commands</summary>
     @hudi-bot supports the following commands:
   
    - `@hudi-bot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [hudi] danny0405 commented on a diff in pull request #8631: [HUDI-6170] Use correct zone id while calculating earliestTimeToRetain

Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8631:
URL: https://github.com/apache/hudi/pull/8631#discussion_r1188063597


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanPlanner.java:
##########
@@ -510,7 +510,7 @@ public Option<HoodieInstant> getEarliestCommitToRetain() {
       }
     } else if (config.getCleanerPolicy() == HoodieCleaningPolicy.KEEP_LATEST_BY_HOURS) {
       Instant instant = Instant.now();
-      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, ZoneId.systemDefault());
+      ZonedDateTime currentDateTime = ZonedDateTime.ofInstant(instant, HoodieInstantTimeGenerator.getTimelineTimeZone().getZoneId());
       String earliestTimeToRetain = HoodieActiveTimeline.formatDate(Date.from(currentDateTime.minusHours(hoursRetained).toInstant()));

Review Comment:
   Yeah, that means the code is prone to making misusages, let's fix all those test falures by initialzing the zoneId manually.



-- 
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-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org