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 11:10:00 UTC

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

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



##########
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:
       Are we sure this is enough? No timezone other than Daylight is causing this edge case miss?




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