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/18 13:11:17 UTC

[GitHub] [hbase] bsglz opened a new pull request #2272: HBASE-24898 Can not set 23:00~24:00 as offpeak hour now

bsglz opened a new pull request #2272:
URL: https://github.com/apache/hbase/pull/2272


   


----------------------------------------------------------------
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] Apache9 commented on a change in pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r473759936



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CurrentHourProvider.java
##########
@@ -54,11 +56,15 @@ private static void moveToNextHour(Calendar calendar) {
 
   public static int getCurrentHour() {
     Tick tick = CurrentHourProvider.tick;
-    if(System.currentTimeMillis() < tick.expirationTimeInMillis) {
+    if (EnvironmentEdgeManager.currentTime() < tick.expirationTimeInMillis) {
       return tick.currentHour;
     }
 
     CurrentHourProvider.tick = tick = nextTick();
     return tick.currentHour;
   }
+
+  protected static void forceUpdateTickForTest() {

Review comment:
       I think this can be done by implementing a special EnvironmentEdge? Where we will return a pre defined sequence of timestamp.




----------------------------------------------------------------
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] Apache9 commented on pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache9 commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-677291674


   But I'm not sure if we could control the return value of CurrentHourProvider? In nextTick method, we will not use EnvironmentEdge.currentTime() so it will still return the current time?


----------------------------------------------------------------
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] Apache9 commented on a change in pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r473790856



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CurrentHourProvider.java
##########
@@ -54,11 +56,15 @@ private static void moveToNextHour(Calendar calendar) {
 
   public static int getCurrentHour() {
     Tick tick = CurrentHourProvider.tick;
-    if(System.currentTimeMillis() < tick.expirationTimeInMillis) {
+    if (EnvironmentEdgeManager.currentTime() < tick.expirationTimeInMillis) {
       return tick.currentHour;
     }
 
     CurrentHourProvider.tick = tick = nextTick();
     return tick.currentHour;
   }
+
+  protected static void forceUpdateTickForTest() {

Review comment:
       You can use any Collections you want. The point here, is that you can inject multiple values at once, and also you can implement more complicated logic in the currentTime method, so you do not need to add a forceUpdateTickForTest method.




----------------------------------------------------------------
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 pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-677353645


   Maybe we could set the time.
   
   >    Calendar calendar = new GregorianCalendar();
   >     calendar.setTimeInMillis(EnvironmentEdgeManager.currentTime());
   


----------------------------------------------------------------
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 #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-677436410


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   6m 57s |  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 41s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m  4s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m  8s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 26s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m  5s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  11m 14s |  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 15s |  The patch does not generate ASF License warnings.  |
   |  |   |  38m 52s |   |
   
   
   | 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-2272/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 43400ebff92e 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 / 1164531d5a |
   | 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-2272/3/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



[GitHub] [hbase] bsglz commented on pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-677365924


   Seems it is ok, will add a unit test for CurrentHourProvider later.


----------------------------------------------------------------
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 #2272: HBASE-24898 Can not set 23:00~24:00 as offpeak hour now

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r472241932



##########
File path: hbase-common/src/main/resources/hbase-default.xml
##########
@@ -917,13 +917,13 @@ possible configurations would overwhelm and obscure the important.
   <property>
     <name>hbase.offpeak.start.hour</name>
     <value>-1</value>
-    <description>The start of off-peak hours, expressed as an integer between 0 and 23, inclusive.
+    <description>The start of off-peak hours, expressed as an integer between 0 and 24, inclusive.
       Set to -1 to disable off-peak.</description>
   </property>
   <property>
     <name>hbase.offpeak.end.hour</name>
     <value>-1</value>
-    <description>The end of off-peak hours, expressed as an integer between 0 and 23, inclusive. Set
+    <description>The end of off-peak hours, expressed as an integer between 0 and 24, exclusive. Set

Review comment:
       `    @Override
       public boolean isOffPeakHour(int targetHour) {
         if (startHour <= endHour) {
           return startHour <= targetHour && targetHour < endHour;
         }
         return targetHour < endHour || startHour <= targetHour;
       }`
   See the code, it will be "return 0 <= hour && hour < 24"
   




----------------------------------------------------------------
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 #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-678043726


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  0s |  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 _ |
   | +0 :ok: |  mvndep  |   7m 35s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 40s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 34s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 48s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 41s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   1m 10s |  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  |  12m 11s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   3m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 21s |  The patch does not generate ASF License warnings.  |
   |  |   |  45m 29s |   |
   
   
   | 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-2272/5/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux b4f391b71bea 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 / 65d28da7c2 |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/5/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | Max. process+thread count | 84 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/5/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



[GitHub] [hbase] Apache9 commented on a change in pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r473802512



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CurrentHourProvider.java
##########
@@ -54,11 +56,15 @@ private static void moveToNextHour(Calendar calendar) {
 
   public static int getCurrentHour() {
     Tick tick = CurrentHourProvider.tick;
-    if(System.currentTimeMillis() < tick.expirationTimeInMillis) {
+    if (EnvironmentEdgeManager.currentTime() < tick.expirationTimeInMillis) {
       return tick.currentHour;
     }
 
     CurrentHourProvider.tick = tick = nextTick();
     return tick.currentHour;
   }
+
+  protected static void forceUpdateTickForTest() {

Review comment:
       Then just make the tick field as package private and add a VisibleForTesting annotation? Just change it directly in UT.




----------------------------------------------------------------
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 #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r473777358



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CurrentHourProvider.java
##########
@@ -54,11 +56,15 @@ private static void moveToNextHour(Calendar calendar) {
 
   public static int getCurrentHour() {
     Tick tick = CurrentHourProvider.tick;
-    if(System.currentTimeMillis() < tick.expirationTimeInMillis) {
+    if (EnvironmentEdgeManager.currentTime() < tick.expirationTimeInMillis) {
       return tick.currentHour;
     }
 
     CurrentHourProvider.tick = tick = nextTick();
     return tick.currentHour;
   }
+
+  protected static void forceUpdateTickForTest() {

Review comment:
       Then we can inject a special value.
   
   >     EnvironmentEdgeManager.injectEdge(()->0L);
   >     assertEquals(0, CurrentHourProvider.getCurrentHour());
   >     EnvironmentEdgeManager.injectEdge(()->15L);
   >     assertEquals(15, CurrentHourProvider.getCurrentHour());




----------------------------------------------------------------
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 #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r473975181



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CurrentHourProvider.java
##########
@@ -54,11 +56,15 @@ private static void moveToNextHour(Calendar calendar) {
 
   public static int getCurrentHour() {
     Tick tick = CurrentHourProvider.tick;
-    if(System.currentTimeMillis() < tick.expirationTimeInMillis) {
+    if (EnvironmentEdgeManager.currentTime() < tick.expirationTimeInMillis) {
       return tick.currentHour;
     }
 
     CurrentHourProvider.tick = tick = nextTick();
     return tick.currentHour;
   }
+
+  protected static void forceUpdateTickForTest() {

Review comment:
       Also the nextTick(), let see the qa result for the recent build first.




----------------------------------------------------------------
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 #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-678238632


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 34s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 27s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 22s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   2m  2s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   3m 37s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 28s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 50s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  14m 27s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   4m  3s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 28s |  The patch does not generate ASF License warnings.  |
   |  |   |  46m 48s |   |
   
   
   | 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-2272/6/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux eb0495c3373c 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 / 54fe81eb56 |
   | Max. process+thread count | 85 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/6/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



[GitHub] [hbase] bsglz edited a comment on pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
bsglz edited a comment on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-677353645


   Maybe we can set the time.
   
   >    Calendar calendar = new GregorianCalendar();
   >     calendar.setTimeInMillis(EnvironmentEdgeManager.currentTime());
   


----------------------------------------------------------------
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 #2272: HBASE-24898 Can not set 23:00~24:00 as offpeak hour now

Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r472236297



##########
File path: hbase-common/src/main/resources/hbase-default.xml
##########
@@ -917,13 +917,13 @@ possible configurations would overwhelm and obscure the important.
   <property>
     <name>hbase.offpeak.start.hour</name>
     <value>-1</value>
-    <description>The start of off-peak hours, expressed as an integer between 0 and 23, inclusive.
+    <description>The start of off-peak hours, expressed as an integer between 0 and 24, inclusive.
       Set to -1 to disable off-peak.</description>
   </property>
   <property>
     <name>hbase.offpeak.end.hour</name>
     <value>-1</value>
-    <description>The end of off-peak hours, expressed as an integer between 0 and 23, inclusive. Set
+    <description>The end of off-peak hours, expressed as an integer between 0 and 24, exclusive. Set

Review comment:
       How is this `exclusive` for 24 based on `return 0 <= hour && hour <= 24; ` condition?




----------------------------------------------------------------
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 #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-678086297


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 28s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 25s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 42s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 20s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 34s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m 10s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 29s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 20s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 20s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 33s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 59s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 20s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 137m 26s |  hbase-server in the patch passed.  |
   |  |   | 165m 24s |   |
   
   
   | 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-2272/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 562ee40a8cfe 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 / 65d28da7c2 |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/5/testReport/ |
   | Max. process+thread count | 4243 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/5/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] Apache-HBase commented on pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-677434755


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 46s |  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  5s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  0s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m  1s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 42s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 41s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 57s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 57s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m  7s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 39s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   6m 46s |  hbase-server in the patch failed.  |
   |  |   |  31m 54s |   |
   
   
   | 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-2272/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 82c739594e74 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 / 1164531d5a |
   | Default Java | 1.8.0_232 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/3/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/3/testReport/ |
   | Max. process+thread count | 587 (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-2272/3/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 commented on a change in pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r473775819



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CurrentHourProvider.java
##########
@@ -54,11 +56,15 @@ private static void moveToNextHour(Calendar calendar) {
 
   public static int getCurrentHour() {
     Tick tick = CurrentHourProvider.tick;
-    if(System.currentTimeMillis() < tick.expirationTimeInMillis) {
+    if (EnvironmentEdgeManager.currentTime() < tick.expirationTimeInMillis) {
       return tick.currentHour;
     }
 
     CurrentHourProvider.tick = tick = nextTick();
     return tick.currentHour;
   }
+
+  protected static void forceUpdateTickForTest() {

Review comment:
       Do you mean like this:
   
   >     if (EnvironmentEdgeManager.currentTime() <= 23) {
   >       return (int)EnvironmentEdgeManager.currentTime();
   >     }




----------------------------------------------------------------
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 #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-678007032


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  4s |  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 45s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 10s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 22s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 41s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 33s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  9s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  9s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 18s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 41s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   9m 30s |  hbase-server in the patch failed.  |
   |  |   |  36m 44s |   |
   
   
   | 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-2272/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 3f7bb69ec499 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 / 65d28da7c2 |
   | Default Java | 2020-01-14 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/4/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-2272/4/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/4/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/4/testReport/ |
   | Max. process+thread count | 505 (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-2272/4/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] Apache-HBase commented on pull request #2272: HBASE-24898 Can not set 23:00~24:00 as offpeak hour now

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-675519077


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   6m 26s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 20s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 59s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   2m 15s |  master passed  |
   | +0 :ok: |  refguide  |   5m 26s |  branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.  |
   | +1 :green_heart: |  spotbugs  |  13m 26s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 41s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   2m 13s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  1s |  The patch has no ill-formed XML file.  |
   | +0 :ok: |  refguide  |   5m 54s |  patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.  |
   | +1 :green_heart: |  hadoopcheck  |  13m  2s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |  12m 57s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 35s |  The patch does not generate ASF License warnings.  |
   |  |   |  78m 26s |   |
   
   
   | 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-2272/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | dupname asflicense refguide xml spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 0722d30dd78e 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 / ea26463a33 |
   | refguide | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/1/artifact/yetus-general-check/output/branch-site/book.html |
   | refguide | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/1/artifact/yetus-general-check/output/patch-site/book.html |
   | Max. process+thread count | 122 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server . U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/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



[GitHub] [hbase] Apache-HBase commented on pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-678382450


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 32s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 27s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 23s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 19s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 32s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 59s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 25s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 28s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 59s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 20s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 137m  6s |  hbase-server in the patch passed.  |
   |  |   | 163m 44s |   |
   
   
   | 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-2272/7/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 7982566b5ecb 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 / 72be041d1c |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/7/testReport/ |
   | Max. process+thread count | 4345 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/7/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] Apache-HBase commented on pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-678301250


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 30s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 22s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 18s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 35s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 53s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 22s |  hbase-common in master failed.  |
   | -0 :warning: |  javadoc  |   0m 50s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 18s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   5m 44s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 55s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 55s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   7m 52s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 19s |  hbase-common in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 48s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 37s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 152m 17s |  hbase-server in the patch passed.  |
   |  |   | 188m  9s |   |
   
   
   | 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-2272/6/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux ceea10cf7ac3 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 54fe81eb56 |
   | Default Java | 2020-01-14 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/6/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/6/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-2272/6/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/6/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-2272/6/testReport/ |
   | Max. process+thread count | 3770 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/6/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] Apache-HBase commented on pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-678408287


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  0s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 21s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 41s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 36s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 26s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 18s |  hbase-common in master failed.  |
   | -0 :warning: |  javadoc  |   0m 41s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 30s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 33s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 33s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 15s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 15s |  hbase-common in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 45s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 194m 27s |  hbase-server in the patch passed.  |
   |  |   | 226m 40s |   |
   
   
   | 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-2272/7/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 9b94aecaa231 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 / 72be041d1c |
   | Default Java | 2020-01-14 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/7/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/7/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-2272/7/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/7/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-2272/7/testReport/ |
   | Max. process+thread count | 2816 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/7/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 commented on a change in pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r473796213



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CurrentHourProvider.java
##########
@@ -54,11 +56,15 @@ private static void moveToNextHour(Calendar calendar) {
 
   public static int getCurrentHour() {
     Tick tick = CurrentHourProvider.tick;
-    if(System.currentTimeMillis() < tick.expirationTimeInMillis) {
+    if (EnvironmentEdgeManager.currentTime() < tick.expirationTimeInMillis) {
       return tick.currentHour;
     }
 
     CurrentHourProvider.tick = tick = nextTick();
     return tick.currentHour;
   }
+
+  protected static void forceUpdateTickForTest() {

Review comment:
       Oh, but i think the test failure above is caused by injectEdge in other test, if EnvironmentEdgeManager.currentTime() < tick.expirationTimeInMillis, then it will return the previous currentHour, it is why i want force update the tick.
   




----------------------------------------------------------------
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 #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-678005066


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 29s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  4s |  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  |   3m 31s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 53s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 36s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 28s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 56s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 56s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 31s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 38s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   6m 27s |  hbase-server in the patch failed.  |
   |  |   |  28m 42s |   |
   
   
   | 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-2272/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 149c101afde6 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 / 65d28da7c2 |
   | Default Java | 1.8.0_232 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/4/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/4/testReport/ |
   | Max. process+thread count | 576 (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-2272/4/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] Apache-HBase commented on pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-676184740


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 36s |  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 41s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  4s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 36s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 47s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  9s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  1s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  1s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 41s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 47s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 213m 19s |  hbase-server in the patch passed.  |
   |  |   | 242m 39s |   |
   
   
   | 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-2272/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 4f0ddbb69e6a 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 / 1164531d5a |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/2/testReport/ |
   | Max. process+thread count | 3634 (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-2272/2/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 commented on a change in pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r474533415



##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/TestCurrentHourProvider.java
##########
@@ -0,0 +1,71 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver.compactions;
+
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.testclassification.RegionServerTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.apache.hadoop.hbase.util.EnvironmentEdge;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category({RegionServerTests.class, SmallTests.class})
+public class TestCurrentHourProvider {
+
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+      HBaseClassTestRule.forClass(TestCurrentHourProvider.class);
+
+  @Test
+  public void testWithEnvironmentEdge() {
+    // set 1597895561000 with timezone GMT+08:00, represent 2020-08-20 11:52:41, should return 11
+    EnvironmentEdge edgeForHour11 = new EnvironmentEdge() {

Review comment:
       Make sense, let me try.




----------------------------------------------------------------
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 #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r473800695



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CurrentHourProvider.java
##########
@@ -54,11 +56,15 @@ private static void moveToNextHour(Calendar calendar) {
 
   public static int getCurrentHour() {
     Tick tick = CurrentHourProvider.tick;
-    if(System.currentTimeMillis() < tick.expirationTimeInMillis) {
+    if (EnvironmentEdgeManager.currentTime() < tick.expirationTimeInMillis) {
       return tick.currentHour;
     }
 
     CurrentHourProvider.tick = tick = nextTick();
     return tick.currentHour;
   }
+
+  protected static void forceUpdateTickForTest() {

Review comment:
       How about the way i mentioned above, a big magic, but seems bring smallest impact.
   
   > if (EnvironmentEdgeManager.currentTime() <= 23) {
   >   return (int)EnvironmentEdgeManager.currentTime();
   > }




----------------------------------------------------------------
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 #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-675913743


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 29s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  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 48s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m  6s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m  3s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 30s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m  1s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  11m 26s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 10s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 14s |  The patch does not generate ASF License warnings.  |
   |  |   |  33m  1s |   |
   
   
   | 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-2272/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux c66a6e9ee745 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 / 1164531d5a |
   | 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-2272/2/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



[GitHub] [hbase] bsglz commented on pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-677324748


   > But I'm not sure if we could control the return value of CurrentHourProvider? In nextTick method, we will not use EnvironmentEdge.currentTime() so it will still return the current time?
   
   Oh, no, seems you are right, the changing is not 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] bsglz commented on a change in pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r473788969



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CurrentHourProvider.java
##########
@@ -54,11 +56,15 @@ private static void moveToNextHour(Calendar calendar) {
 
   public static int getCurrentHour() {
     Tick tick = CurrentHourProvider.tick;
-    if(System.currentTimeMillis() < tick.expirationTimeInMillis) {
+    if (EnvironmentEdgeManager.currentTime() < tick.expirationTimeInMillis) {
       return tick.currentHour;
     }
 
     CurrentHourProvider.tick = tick = nextTick();
     return tick.currentHour;
   }
+
+  protected static void forceUpdateTickForTest() {

Review comment:
       A bit confusion of why we need a queue?




----------------------------------------------------------------
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 closed pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
virajjasani closed pull request #2272:
URL: https://github.com/apache/hbase/pull/2272


   


----------------------------------------------------------------
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 #2272: HBASE-24898 Can not set 23:00~24:00 as offpeak hour now

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r472241932



##########
File path: hbase-common/src/main/resources/hbase-default.xml
##########
@@ -917,13 +917,13 @@ possible configurations would overwhelm and obscure the important.
   <property>
     <name>hbase.offpeak.start.hour</name>
     <value>-1</value>
-    <description>The start of off-peak hours, expressed as an integer between 0 and 23, inclusive.
+    <description>The start of off-peak hours, expressed as an integer between 0 and 24, inclusive.
       Set to -1 to disable off-peak.</description>
   </property>
   <property>
     <name>hbase.offpeak.end.hour</name>
     <value>-1</value>
-    <description>The end of off-peak hours, expressed as an integer between 0 and 23, inclusive. Set
+    <description>The end of off-peak hours, expressed as an integer between 0 and 24, exclusive. Set

Review comment:
       >     @Override
   >     public boolean isOffPeakHour(int targetHour) {
   >       if (startHour <= endHour) {
   >         return startHour <= targetHour && targetHour < endHour;
   >       }
   >       return targetHour < endHour || startHour <= targetHour;
   >     }
   
   See the code, it will be "return 0 <= hour && hour < 24"
   




----------------------------------------------------------------
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] Apache9 commented on a change in pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r474455611



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/util/EnvironmentEdge.java
##########
@@ -34,4 +36,13 @@
    * @return Current time.
    */
   long currentTime();
+
+  /**
+   * Returns the currentTimeZone.
+   *
+   * @return Current timezone.
+   */
+  default TimeZone currentTimeZone() {

Review comment:
       A timestamp does not need TimeZone I assume?

##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/TestCurrentHourProvider.java
##########
@@ -0,0 +1,71 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver.compactions;
+
+import java.util.TimeZone;
+
+import static org.junit.Assert.assertEquals;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.testclassification.RegionServerTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.apache.hadoop.hbase.util.EnvironmentEdge;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category({RegionServerTests.class, SmallTests.class})
+public class TestCurrentHourProvider {
+
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+      HBaseClassTestRule.forClass(TestCurrentHourProvider.class);
+
+  @Test
+  public void testWithEnvironmentEdge() {
+    // set 1597895561000 with timezone GMT+08:00, represent 2020-08-20 11:52:41, should return 11
+    EnvironmentEdge edgeForHour11 = new EnvironmentEdge() {

Review comment:
       I would like to do this in a oppsite way, that is, based on the current time zone, we adjust the currentTime, so we will return the same hour.
   In general, we should try our best to not add additional logic to normal code only if we want to write a UT.




----------------------------------------------------------------
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 #2272: HBASE-24898 Can not set 23:00~24:00 as offpeak hour now

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-675662971


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 27s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 23s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 34s |  master passed  |
   | +1 :green_heart: |  compile  |   2m 19s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 39s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   2m 55s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 17s |  the patch passed  |
   | +1 :green_heart: |  compile  |   2m 16s |  the patch passed  |
   | +1 :green_heart: |  javac  |   2m 16s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 35s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   2m 54s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 320m 30s |  root in the patch failed.  |
   |  |   | 352m 36s |   |
   
   
   | 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-2272/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 698ce5c67287 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 / ea26463a33 |
   | Default Java | 1.8.0_232 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/1/testReport/ |
   | Max. process+thread count | 5535 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server . U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/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] Apache-HBase commented on pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-678287488


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 38s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 24s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 36s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   7m  6s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 59s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 37s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 18s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 18s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 35s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 57s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 25s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 141m  0s |  hbase-server in the patch passed.  |
   |  |   | 171m 44s |   |
   
   
   | 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-2272/6/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 85e4db2251d7 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 / 54fe81eb56 |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/6/testReport/ |
   | Max. process+thread count | 4296 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/6/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] Apache-HBase commented on pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-678006758


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  4s |  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 54s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 10s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m  4s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 43s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 11s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  12m 11s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 14s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 12s |  The patch does not generate ASF License warnings.  |
   |  |   |  35m  7s |   |
   
   
   | 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-2272/4/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 4fbdf49bca41 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 / 65d28da7c2 |
   | Max. process+thread count | 84 (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-2272/4/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



[GitHub] [hbase] bsglz commented on pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-678032130


   Digged more, and i think the failure in fact caused by timezone, let see the new build for verify.


----------------------------------------------------------------
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 #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-678321644


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  2s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 50s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 34s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 51s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 11s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 42s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 33s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  12m 12s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   3m  7s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 21s |  The patch does not generate ASF License warnings.  |
   |  |   |  38m 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-2272/7/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 00eefe3bc54c 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 / 72be041d1c |
   | Max. process+thread count | 84 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/7/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



[GitHub] [hbase] Apache-HBase commented on pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-678089834


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 30s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 24s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 28s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 33s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m  7s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 20s |  hbase-common in master failed.  |
   | -0 :warning: |  javadoc  |   0m 42s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 20s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 40s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 40s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m  9s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 18s |  hbase-common in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 40s |  hbase-common in the patch passed.  |
   | +1 :green_heart: |  unit  | 142m 44s |  hbase-server in the patch passed.  |
   |  |   | 174m  4s |   |
   
   
   | 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-2272/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 469d322fc846 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 / 65d28da7c2 |
   | Default Java | 2020-01-14 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/5/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/5/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-2272/5/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/5/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-2272/5/testReport/ |
   | Max. process+thread count | 3816 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/5/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] Apache-HBase commented on pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-676074974


   :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 24s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 12s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 10s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 42s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  3s |  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  |   5m 51s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 133m 23s |  hbase-server in the patch passed.  |
   |  |   | 159m 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-2272/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux b2213e54a8d8 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 / 1164531d5a |
   | Default Java | 2020-01-14 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/2/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-2272/2/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-2272/2/testReport/ |
   | Max. process+thread count | 4231 (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-2272/2/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 commented on a change in pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r473800695



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CurrentHourProvider.java
##########
@@ -54,11 +56,15 @@ private static void moveToNextHour(Calendar calendar) {
 
   public static int getCurrentHour() {
     Tick tick = CurrentHourProvider.tick;
-    if(System.currentTimeMillis() < tick.expirationTimeInMillis) {
+    if (EnvironmentEdgeManager.currentTime() < tick.expirationTimeInMillis) {
       return tick.currentHour;
     }
 
     CurrentHourProvider.tick = tick = nextTick();
     return tick.currentHour;
   }
+
+  protected static void forceUpdateTickForTest() {

Review comment:
       How about the way i mentioned above, a bit magic, but seems bring smallest impact.
   
   > if (EnvironmentEdgeManager.currentTime() <= 23) {
   >   return (int)EnvironmentEdgeManager.currentTime();
   > }




----------------------------------------------------------------
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 #2272: HBASE-24898 Can not set 23:00~24:00 as offpeak hour now

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-675627638


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   2m 16s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 20s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 38s |  master passed  |
   | +1 :green_heart: |  compile  |   3m  2s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 20s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 15s |  root in master failed.  |
   | -0 :warning: |  javadoc  |   0m 15s |  hbase-common in master failed.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 29s |  the patch passed  |
   | +1 :green_heart: |  compile  |   3m  7s |  the patch passed  |
   | +1 :green_heart: |  javac  |   3m  7s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 45s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 20s |  hbase-common in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 47s |  hbase-server in the patch failed.  |
   | -0 :warning: |  javadoc  |   0m 19s |  root in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 242m 12s |  root in the patch passed.  |
   |  |   | 278m  5s |   |
   
   
   | 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-2272/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 04ffc70b2c56 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 / ea26463a33 |
   | Default Java | 2020-01-14 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-root.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/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-2272/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/1/testReport/ |
   | Max. process+thread count | 4889 (vs. ulimit of 12500) |
   | modules | C: hbase-common hbase-server . U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/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] Apache9 commented on a change in pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#discussion_r473780685



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CurrentHourProvider.java
##########
@@ -54,11 +56,15 @@ private static void moveToNextHour(Calendar calendar) {
 
   public static int getCurrentHour() {
     Tick tick = CurrentHourProvider.tick;
-    if(System.currentTimeMillis() < tick.expirationTimeInMillis) {
+    if (EnvironmentEdgeManager.currentTime() < tick.expirationTimeInMillis) {
       return tick.currentHour;
     }
 
     CurrentHourProvider.tick = tick = nextTick();
     return tick.currentHour;
   }
+
+  protected static void forceUpdateTickForTest() {

Review comment:
       ```
   class MockEnvironmentEdge implements EnvironmentEdge {
     private final Queue<Long> times = new ArrayDeque<>();
   
     public void inject(long timestamp) {
       times.addLast(timestamp);
     }
   
     public long currentTime() {
       return times.pollFirst();
     }
   }
   
   EnvironmentEdgeManager.injectEdge(new MockEnvironmentEdge());
   ```
   
   Something like this.




----------------------------------------------------------------
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 pull request #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-677994326


   Seems QA stuck in build#3, can you help to stop it? Thanks. @Apache9 


----------------------------------------------------------------
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 #2272: HBASE-24898 Use EnvironmentEdge.currentTime() instead of System.currentTimeMillis() in CurrentHourProvider

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2272:
URL: https://github.com/apache/hbase/pull/2272#issuecomment-677435356


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 29s |  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 26s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 12s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 58s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 43s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  3s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  7s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  7s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 50s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   7m  4s |  hbase-server in the patch failed.  |
   |  |   |  32m 46s |   |
   
   
   | 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-2272/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2272 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 585cbca30603 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 1164531d5a |
   | Default Java | 2020-01-14 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/3/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-2272/3/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/3/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2272/3/testReport/ |
   | Max. process+thread count | 764 (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-2272/3/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