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 07:14:32 UTC
[GitHub] [hbase] bsglz opened a new pull request #2307: HBASE-24947 Addendum for HBASE-24898 to deal with Daylight Saving Time
bsglz opened a new pull request #2307:
URL: https://github.com/apache/hbase/pull/2307
----------------------------------------------------------------
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
[GitHub] [hbase] bsglz commented on a change in pull request #2307: HBASE-24947 Addendum for HBASE-24898 to deal with Daylight Saving Time
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [hbase] Apache-HBase commented on pull request #2307: HBASE-24947 Addendum for HBASE-24898 to deal with Daylight Saving Time
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2307:
URL: https://github.com/apache/hbase/pull/2307#issuecomment-679964210
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 3s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 8s | master passed |
| +1 :green_heart: | compile | 1m 1s | master passed |
| +1 :green_heart: | shadedjars | 7m 9s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 49s | the patch passed |
| +1 :green_heart: | compile | 0m 59s | the patch passed |
| +1 :green_heart: | javac | 0m 59s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 13s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 215m 21s | hbase-server in the patch passed. |
| | | 244m 31s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2307/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2307 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 0c2693e1b0f8 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1220a8775c |
| Default Java | 1.8.0_232 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2307/1/testReport/ |
| Max. process+thread count | 3413 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2307/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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
[GitHub] [hbase] bsglz closed pull request #2307: HBASE-24947 Addendum for HBASE-24898 to deal with Daylight Saving Time
Posted by GitBox <gi...@apache.org>.
bsglz closed pull request #2307:
URL: https://github.com/apache/hbase/pull/2307
----------------------------------------------------------------
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
[GitHub] [hbase] Apache-HBase commented on pull request #2307: HBASE-24947 Addendum for HBASE-24898 to deal with Daylight Saving Time
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2307:
URL: https://github.com/apache/hbase/pull/2307#issuecomment-679930191
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 27s | master passed |
| +1 :green_heart: | compile | 1m 9s | master passed |
| +1 :green_heart: | shadedjars | 6m 48s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 44s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 4s | the patch passed |
| +1 :green_heart: | compile | 1m 6s | the patch passed |
| +1 :green_heart: | javac | 1m 6s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 34s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 39s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 137m 56s | hbase-server in the patch passed. |
| | | 166m 22s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2307/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2307 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 0633da9e4499 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1220a8775c |
| Default Java | 2020-01-14 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2307/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2307/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2307/1/testReport/ |
| Max. process+thread count | 4169 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2307/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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
[GitHub] [hbase] virajjasani commented on a change in pull request #2307: HBASE-24947 Addendum for HBASE-24898 to deal with Daylight Saving Time
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [hbase] Apache-HBase commented on pull request #2307: HBASE-24947 Addendum for HBASE-24898 to deal with Daylight Saving Time
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2307:
URL: https://github.com/apache/hbase/pull/2307#issuecomment-679864158
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 59s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 58s | master passed |
| +1 :green_heart: | checkstyle | 1m 9s | master passed |
| +1 :green_heart: | spotbugs | 2m 3s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 28s | the patch passed |
| -0 :warning: | checkstyle | 1m 5s | hbase-server: The patch generated 1 new + 3 unchanged - 0 fixed = 4 total (was 3) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 25s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 8s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 16s | The patch does not generate ASF License warnings. |
| | | 33m 55s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2307/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2307 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 430d6832ee43 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1220a8775c |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2307/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 94 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2307/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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