You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/08/25 13:19:08 UTC

[GitHub] [hbase] bsglz commented on a change in pull request #2307: HBASE-24947 Addendum for HBASE-24898 to deal with Daylight Saving Time

bsglz commented on a change in pull request #2307:
URL: https://github.com/apache/hbase/pull/2307#discussion_r476441331



##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/TestCurrentHourProvider.java
##########
@@ -42,19 +40,43 @@
    * and the unix time of 2020-08-20 15:04:00 is 1597907081000,
    * by calculating the delta time to get expected time in current timezone,
    * then we can get special hour no matter which timezone it runs.
+   *
+   * In addition, we should consider the Daylight Saving Time.
+   * 1. If in DaylightTime, we need reduce one hour.
+   * 2. If in DaylightTime and timezone is "Antarctica/Troll", we need reduce two hours.
    */
   @Test
   public void testWithEnvironmentEdge() {
-    // set a time represent hour 11
-    long deltaFor11 = TimeZone.getDefault().getRawOffset() - 28800000;
-    long timeFor11 = 1597895561000L - deltaFor11;
-    EnvironmentEdgeManager.injectEdge(() -> timeFor11);
-    assertEquals(11, CurrentHourProvider.getCurrentHour());
+    // test for all available zoneID
+    for (String zoneID : TimeZone.getAvailableIDs()) {
+      TimeZone timezone = TimeZone.getTimeZone(zoneID);
+      TimeZone.setDefault(timezone);
 
-    // set a time represent hour 15
-    long deltaFor15 = TimeZone.getDefault().getRawOffset() - 28800000;
-    long timeFor15 = 1597907081000L - deltaFor15;
-    EnvironmentEdgeManager.injectEdge(() -> timeFor15);
-    assertEquals(15, CurrentHourProvider.getCurrentHour());
+      // set a time represent hour 11
+      long deltaFor11 = TimeZone.getDefault().getRawOffset() - 28800000;
+      long timeFor11 = 1597895561000L - deltaFor11;
+      EnvironmentEdgeManager.injectEdge(() -> timeFor11);
+      CurrentHourProvider.tick = CurrentHourProvider.nextTick();
+      int hour11 = CurrentHourProvider.getCurrentHour();
+      if (TimeZone.getDefault().inDaylightTime(new Date(timeFor11))) {
+        hour11 = "Antarctica/Troll".equals(zoneID) ?
+          CurrentHourProvider.getCurrentHour() - 2 :
+          CurrentHourProvider.getCurrentHour() - 1;
+      }
+      assertEquals(11, hour11);
+
+      // set a time represent hour 15
+      long deltaFor15 = TimeZone.getDefault().getRawOffset() - 28800000;
+      long timeFor15 = 1597907081000L - deltaFor15;
+      EnvironmentEdgeManager.injectEdge(() -> timeFor15);
+      CurrentHourProvider.tick = CurrentHourProvider.nextTick();
+      int hour15 = CurrentHourProvider.getCurrentHour();
+      if (TimeZone.getDefault().inDaylightTime(new Date(timeFor15))) {
+        hour15 = "Antarctica/Troll".equals(zoneID) ?

Review comment:
       Yeah, for special datetime such as 2020-08-20 11:52:41 and 2020-08-20 15:04:00, i think so, but not sure if there are different calculation for other dates. 
   Considering this case is target to test if the EnvironmentEdge take effect correctly in CurrentHourProvider, maybe it is enough?




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

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