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 2021/06/23 06:31:43 UTC

[GitHub] [hbase] clarax opened a new pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

clarax opened a new pull request #3415:
URL: https://github.com/apache/hbase/pull/3415


   


-- 
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] clarax commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   I don't understand why there is the Javac complaint. I didn't even touch the file.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 31s |  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  1s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 20s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 18s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 17s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 45s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 20s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 20s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 13s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   7m 47s |  hbase-balancer in the patch passed.  |
   |  |   |  35m  3s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/7/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 9210533e07f2 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 / bb84892550 |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/7/testReport/ |
   | Max. process+thread count | 354 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/7/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 31s |  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 25s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 27s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 17s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 23s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 26s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 17s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |  15m 42s |  hbase-balancer in the patch failed.  |
   |  |   |  44m 28s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux df1b650922ea 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 / fa2d127b7f |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/3/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-balancer.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/3/testReport/ |
   | Max. process+thread count | 315 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/3/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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 #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  3s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 58s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 35s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 17s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   0m 36s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 36s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 32s |  the patch passed  |
   | -0 :warning: |  javac  |   0m 32s |  hbase-balancer generated 1 new + 17 unchanged - 0 fixed = 18 total (was 17)  |
   | -0 :warning: |  checkstyle  |   0m 15s |  hbase-balancer: The patch generated 1 new + 7 unchanged - 0 fixed = 8 total (was 7)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  18m 11s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   0m 44s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 14s |  The patch does not generate ASF License warnings.  |
   |  |   |  37m 50s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/5/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 7227782187c7 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 / dcd0fb8b1d |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/5/artifact/yetus-general-check/output/diff-compile-javac-hbase-balancer.txt |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/5/artifact/yetus-general-check/output/diff-checkstyle-hbase-balancer.txt |
   | Max. process+thread count | 96 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/5/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.12.0 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] clarax commented on a change in pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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



##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/DoubleArrayCost.java
##########
@@ -106,4 +88,32 @@ private static double getSum(double[] stats) {
     }
     return total;
   }
+
+  /**
+   * Return the min skew of distribution
+   */
+  public static double getMinSkew(double total, double numServers) {

Review comment:
       yes.




-- 
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] ndimiduk commented on a change in pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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



##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/DoubleArrayCost.java
##########
@@ -106,4 +88,32 @@ private static double getSum(double[] stats) {
     }
     return total;
   }
+
+  /**
+   * Return the min skew of distribution
+   */
+  public static double getMinSkew(double total, double numServers) {

Review comment:
       Why are the input arguments `double`? Can there be a fractional amount of either of these quantities?

##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerClusterState.java
##########
@@ -301,17 +307,28 @@ public String getRack(ServerName server) {
     for (int i = 0; i < regionIndexToServerIndex.length; i++) {
       if (regionIndexToServerIndex[i] >= 0) {
         numRegionsPerServerPerTable[regionIndexToServerIndex[i]][regionIndexToTableIndex[i]]++;
+        numRegionsPerTable[regionIndexToTableIndex[i]]++;
       }
     }
 
-    numMaxRegionsPerTable = new int[numTables];
+    // Avoid repeated computation for planning
+    meanRegionsPerTable = new double[numTables];
+    regionSkewByTable = new double[numTables];
+    maxRegionSkewByTable  = new double[numTables];
+    minRegionSkewByTable = new double[numTables];
+
+    for (int i = 0; i < numTables; i++) {
+      meanRegionsPerTable[i] = Double.valueOf(numRegionsPerTable[i]) / numServers;
+      minRegionSkewByTable[i] += DoubleArrayCost.getMinSkew(numRegionsPerTable[i], numServers);
+      maxRegionSkewByTable[i] += DoubleArrayCost.getMaxSkew(numRegionsPerTable[i], numServers);
+    }
+
     for (int[] aNumRegionsPerServerPerTable : numRegionsPerServerPerTable) {
-      for (tableIndex = 0; tableIndex < aNumRegionsPerServerPerTable.length; tableIndex++) {
-        if (aNumRegionsPerServerPerTable[tableIndex] > numMaxRegionsPerTable[tableIndex]) {
-          numMaxRegionsPerTable[tableIndex] = aNumRegionsPerServerPerTable[tableIndex];
-        }
+      for (int tableIdx = 0; tableIdx < aNumRegionsPerServerPerTable.length; tableIdx++) {
+        regionSkewByTable[tableIdx] += Math.abs(aNumRegionsPerServerPerTable[tableIdx]
+          - meanRegionsPerTable[tableIdx]);
       }
-    }
+     }

Review comment:
       nit: white space.

##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/DoubleArrayCost.java
##########
@@ -106,4 +88,32 @@ private static double getSum(double[] stats) {
     }
     return total;
   }
+
+  /**
+   * Return the min skew of distribution
+   */
+  public static double getMinSkew(double total, double numServers) {

Review comment:
       Is `total` the "total number of regions in the cluster"?

##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/DoubleArrayCost.java
##########
@@ -106,4 +88,32 @@ private static double getSum(double[] stats) {
     }
     return total;
   }
+
+  /**
+   * Return the min skew of distribution
+   */
+  public static double getMinSkew(double total, double numServers) {
+    double mean = total / numServers;
+    // It's possible that there aren't enough regions to go around
+    double min;
+    if (numServers > total) {

Review comment:
       should this be `>=` ?

##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -240,7 +240,8 @@ protected void loadConf(Configuration conf) {
     curFunctionCosts = new double[costFunctions.size()];
     tempFunctionCosts = new double[costFunctions.size()];
 
-    LOG.info("Loaded config; maxSteps=" + maxSteps + ", stepsPerRegion=" + stepsPerRegion +
+    LOG.info("Loaded config; maxSteps=" + maxSteps + " ,runMaxSteps=" + runMaxSteps,

Review comment:
       nit: white space.

##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/CostFunction.java
##########
@@ -89,13 +89,14 @@ protected void regionMoved(int region, int oldServer, int newServer) {
    * @return The scaled value.
    */
   protected static double scale(double min, double max, double value) {
-    if (max <= min || value <= min) {
+    if (max <= min || value <= min
+      || Math.abs(max - min) <= 0.01 || Math.abs(value - min) <= 0.01) {

Review comment:
       values like `minCostNeedBalance` below are at 0.00 precision, so I think we should have an epsilon of at least 0.000 precision.




-- 
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 #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  8s |  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  |   4m  2s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 29s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 14s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   0m 32s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  2s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  the patch passed  |
   | -0 :warning: |  javac  |   0m 30s |  hbase-balancer generated 1 new + 17 unchanged - 0 fixed = 18 total (was 17)  |
   | -0 :warning: |  checkstyle  |   0m 13s |  hbase-balancer: The patch generated 1 new + 7 unchanged - 0 fixed = 8 total (was 7)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  19m 49s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   0m 45s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 10s |  The patch does not generate ASF License warnings.  |
   |  |   |  40m 13s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/6/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 4ae3c7616ade 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / d11dc81721 |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/6/artifact/yetus-general-check/output/diff-compile-javac-hbase-balancer.txt |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/6/artifact/yetus-general-check/output/diff-checkstyle-hbase-balancer.txt |
   | Max. process+thread count | 86 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/6/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.12.0 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] clarax commented on a change in pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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



##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/CostFunction.java
##########
@@ -89,13 +89,14 @@ protected void regionMoved(int region, int oldServer, int newServer) {
    * @return The scaled value.
    */
   protected static double scale(double min, double max, double value) {
-    if (max <= min || value <= min) {
+    if (max <= min || value <= min
+      || Math.abs(max - min) <= 0.01 || Math.abs(value - min) <= 0.01) {

Review comment:
       Let me create a COST_EPSILON because I have seen quite wide range of precision.




-- 
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] clarax commented on a change in pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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



##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/DoubleArrayCost.java
##########
@@ -72,31 +72,13 @@ private static double computeCost(double[] stats) {
     double count = stats.length;
     double mean = total / count;
 
-    // Compute max as if all region servers had 0 and one had the sum of all costs. This must be
-    // a zero sum cost for this to make sense.
-    double max = ((count - 1) * mean) + (total - mean);
-
-    // It's possible that there aren't enough regions to go around
-    double min;
-    if (count > total) {
-      min = ((count - total) * mean) + ((1 - mean) * total);
-    } else {
-      // Some will have 1 more than everything else.
-      int numHigh = (int) (total - (Math.floor(mean) * count));
-      int numLow = (int) (count - numHigh);
-
-      min = (numHigh * (Math.ceil(mean) - mean)) + (numLow * (mean - Math.floor(mean)));
-
-    }
-    min = Math.max(0, min);

Review comment:
       applied later by prior code change.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache9 commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   The error prone output is not very stable, so it is just a warning, not a blocker, unless error prone fails the compliation.
   
   Just go ahead.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :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 _ |
   | +1 :green_heart: |  mvninstall  |   3m 59s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 11s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 18s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 41s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 11s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |  16m 51s |  hbase-balancer in the patch failed.  |
   |  |   |  43m 56s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux f8373defdd0a 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 / dcd0fb8b1d |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/4/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-balancer.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/4/testReport/ |
   | Max. process+thread count | 355 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/4/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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 #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 36s |  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  |   4m 48s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 39s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 16s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   0m 38s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m  9s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 36s |  the patch passed  |
   | -0 :warning: |  javac  |   0m 36s |  hbase-balancer generated 1 new + 17 unchanged - 0 fixed = 18 total (was 17)  |
   | -0 :warning: |  checkstyle  |   0m 15s |  hbase-balancer: The patch generated 1 new + 7 unchanged - 0 fixed = 8 total (was 7)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  18m 37s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   0m 46s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 12s |  The patch does not generate ASF License warnings.  |
   |  |   |  41m 45s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 113bcd37b139 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 / fa2d127b7f |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/3/artifact/yetus-general-check/output/diff-compile-javac-hbase-balancer.txt |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/3/artifact/yetus-general-check/output/diff-checkstyle-hbase-balancer.txt |
   | Max. process+thread count | 96 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/3/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.12.0 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 #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 31s |  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  |   3m 47s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 20s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 13s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 50s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 14s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |  15m 27s |  hbase-balancer in the patch failed.  |
   |  |   |  42m 29s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 32ab656c9e06 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 / fa2d127b7f |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/2/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-balancer.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/2/testReport/ |
   | Max. process+thread count | 355 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/2/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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] clarax commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   @ndimiduk @Apache9 Good to go?
   


-- 
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] clarax commented on a change in pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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



##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -125,7 +125,7 @@
   private int stepsPerRegion = 800;
   private long maxRunningTime = 30 * 1000 * 1; // 30 seconds.
   private int numRegionLoadsToRemember = 15;
-  private float minCostNeedBalance = 0.05f;
+  private float minCostNeedBalance = 0.025f;

Review comment:
       Yes, as explained in the jira and the pr, we have to lower it for consistent user experience. the old code almost always return max for table skew that inflates the total cost.




-- 
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 #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   3m 49s |  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 31s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 12s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 21s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 14s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 11s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 18s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |  40m 37s |  hbase-balancer in the patch failed.  |
   |  |   |  72m 11s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/8/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux e11921cf8c12 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 / 1e763d521f |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/8/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-balancer.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/8/testReport/ |
   | Max. process+thread count | 328 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/8/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] saintstack merged pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

Posted by GitBox <gi...@apache.org>.
saintstack merged pull request #3415:
URL: https://github.com/apache/hbase/pull/3415


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 44s |  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  |   4m 52s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 38s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 16s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   0m 39s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 50s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 37s |  the patch passed  |
   | -0 :warning: |  javac  |   0m 37s |  hbase-balancer generated 1 new + 17 unchanged - 0 fixed = 18 total (was 17)  |
   | -0 :warning: |  checkstyle  |   0m 17s |  hbase-balancer: The patch generated 1 new + 7 unchanged - 0 fixed = 8 total (was 7)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  25m  2s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   0m 58s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 13s |  The patch does not generate ASF License warnings.  |
   |  |   |  49m 54s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 4edfb2914e4e 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 / fa2d127b7f |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/2/artifact/yetus-general-check/output/diff-compile-javac-hbase-balancer.txt |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-balancer.txt |
   | Max. process+thread count | 96 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/2/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.12.0 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 #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :confetti_ball: **+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  |   3m 35s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 20s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 10s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 40s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 23s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 23s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 15s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   7m 24s |  hbase-balancer in the patch passed.  |
   |  |   |  34m  4s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 541f14ce6119 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 / dcd0fb8b1d |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/5/testReport/ |
   | Max. process+thread count | 355 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/5/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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] clarax commented on a change in pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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



##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -125,7 +125,7 @@
   private int stepsPerRegion = 800;
   private long maxRunningTime = 30 * 1000 * 1; // 30 seconds.
   private int numRegionLoadsToRemember = 15;
-  private float minCostNeedBalance = 0.05f;
+  private float minCostNeedBalance = 0.025f;

Review comment:
       Yes, as explained in the jira and the pr, we have to lower it for constant user experience. the old code almost always return max for table skew that inflates the total cost.




-- 
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 #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :confetti_ball: **+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 _ |
   | +1 :green_heart: |  mvninstall  |   4m 20s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 23s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 11s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 18s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 16s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 13s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 18s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   8m 58s |  hbase-balancer in the patch passed.  |
   |  |   |  37m  2s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/9/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 48f7d63c2e49 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 / 1e763d521f |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/9/testReport/ |
   | Max. process+thread count | 311 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/9/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 31s |  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 52s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 36s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 16s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   0m 38s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 38s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  the patch passed  |
   | -0 :warning: |  javac  |   0m 30s |  hbase-balancer generated 1 new + 17 unchanged - 0 fixed = 18 total (was 17)  |
   | +1 :green_heart: |  checkstyle  |   0m 14s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  18m 29s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   0m 44s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 14s |  The patch does not generate ASF License warnings.  |
   |  |   |  37m 34s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/7/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 73c38007d7ba 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 / bb84892550 |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/7/artifact/yetus-general-check/output/diff-compile-javac-hbase-balancer.txt |
   | Max. process+thread count | 96 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/7/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :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 _ |
   | +1 :green_heart: |  mvninstall  |   3m 36s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 14s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 38s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 11s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   7m 41s |  hbase-balancer in the patch passed.  |
   |  |   |  34m 21s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/9/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 0149eb869955 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 / 1e763d521f |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/9/testReport/ |
   | Max. process+thread count | 354 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/9/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] clarax commented on a change in pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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



##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/DoubleArrayCost.java
##########
@@ -106,4 +88,32 @@ private static double getSum(double[] stats) {
     }
     return total;
   }
+
+  /**
+   * Return the min skew of distribution
+   */
+  public static double getMinSkew(double total, double numServers) {

Review comment:
       It is to convert the input from integer to double for computation in the function. 




-- 
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 #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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



##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/CostFunction.java
##########
@@ -89,13 +89,14 @@ protected void regionMoved(int region, int oldServer, int newServer) {
    * @return The scaled value.
    */
   protected static double scale(double min, double max, double value) {
-    if (max <= min || value <= min) {
+    if (max <= min || value <= min
+      || Math.abs(max - min) <= 0.01 || Math.abs(value - min) <= 0.01) {

Review comment:
       Is 0.01 a suitable threshold? At least make a constant maybe called `DELTA`?

##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
##########
@@ -125,7 +125,7 @@
   private int stepsPerRegion = 800;
   private long maxRunningTime = 30 * 1000 * 1; // 30 seconds.
   private int numRegionLoadsToRemember = 15;
-  private float minCostNeedBalance = 0.05f;
+  private float minCostNeedBalance = 0.025f;

Review comment:
       Is this change related?

##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerClusterState.java
##########
@@ -290,7 +294,9 @@ public String getRack(ServerName server) {
     }
 
     numTables = tables.size();
+    LOG.info("number of tables = {}", numTables);

Review comment:
       Is info the suitable level here? Or debug?




-- 
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 #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :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  |   4m 19s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 32s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 14s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   0m 35s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  0s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 29s |  the patch passed  |
   | -0 :warning: |  javac  |   0m 29s |  hbase-balancer generated 1 new + 17 unchanged - 0 fixed = 18 total (was 17)  |
   | -0 :warning: |  checkstyle  |   0m 12s |  hbase-balancer: The patch generated 1 new + 7 unchanged - 0 fixed = 8 total (was 7)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  19m 57s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   0m 43s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 12s |  The patch does not generate ASF License warnings.  |
   |  |   |  40m 41s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/4/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux e450ef69683e 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / dcd0fb8b1d |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/4/artifact/yetus-general-check/output/diff-compile-javac-hbase-balancer.txt |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/4/artifact/yetus-general-check/output/diff-checkstyle-hbase-balancer.txt |
   | Max. process+thread count | 86 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/4/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.12.0 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 #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :confetti_ball: **+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 14s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m  9s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 18s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 11s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m  9s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 17s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   8m 13s |  hbase-balancer in the patch passed.  |
   |  |   |  36m  0s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/6/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux bea4020a84a1 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 / d11dc81721 |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/6/testReport/ |
   | Max. process+thread count | 313 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/6/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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] saintstack commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   Merged. All unit tests passed. The complaint was from cleanup post-build messaging H2 machine.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 31s |  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  |   3m 42s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 18s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m  7s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 45s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 20s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 20s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 10s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |  15m 47s |  hbase-balancer in the patch failed.  |
   |  |   |  42m 25s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 5655393aecf6 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 / fa2d127b7f |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/3/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-balancer.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/3/testReport/ |
   | Max. process+thread count | 355 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/3/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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 #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :confetti_ball: **+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 _ |
   | +1 :green_heart: |  mvninstall  |   4m 30s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 15s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 21s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 19s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m  8s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 18s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   8m 14s |  hbase-balancer in the patch passed.  |
   |  |   |  36m 28s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux d4c3c1ad5fdf 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 / dcd0fb8b1d |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/5/testReport/ |
   | Max. process+thread count | 320 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/5/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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 pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   Please fix the checkstyle issue before merging.
   
   Thanks.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] clarax commented on a change in pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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



##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/DoubleArrayCost.java
##########
@@ -106,4 +88,32 @@ private static double getSum(double[] stats) {
     }
     return total;
   }
+
+  /**
+   * Return the min skew of distribution
+   */
+  public static double getMinSkew(double total, double numServers) {
+    double mean = total / numServers;
+    // It's possible that there aren't enough regions to go around
+    double min;
+    if (numServers > total) {

Review comment:
       This is the case when we have more nodes than regions, we will have nodes without regions and it is balanced.




-- 
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 #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  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 12s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 52s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 18s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  8s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 45s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |  21m 58s |  hbase-balancer in the patch failed.  |
   |  |   |  51m  3s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 0271f67dc3da 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 / 9a324bd4d0 |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-balancer.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/1/testReport/ |
   | Max. process+thread count | 355 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/1/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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] clarax commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   Enabling runMaxStep and increasing max run time for TestStochasticBalancerLargeCluster takes care of the flaky tests. I removed the increase for max run time for TestStochasticBalancerBalanceCluster since it is not really needed and increase total rest run 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] Apache-HBase commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :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 _ |
   | +1 :green_heart: |  mvninstall  |   4m 18s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 11s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 18s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 16s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 15s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 18s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  14m 16s |  hbase-balancer in the patch passed.  |
   |  |   |  42m 23s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 14d22bf0b811 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 / dcd0fb8b1d |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/4/testReport/ |
   | Max. process+thread count | 323 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/4/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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 #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 30s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  2s |  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 36s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 28s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 20s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 19s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 22s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 18s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 18s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |  19m 16s |  hbase-balancer in the patch failed.  |
   |  |   |  48m  5s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux a6ede9294c99 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 / 9a324bd4d0 |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-balancer.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/1/testReport/ |
   | Max. process+thread count | 314 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/1/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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 #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  7s |  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  |   4m  4s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 29s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 12s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   0m 32s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  1s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 29s |  the patch passed  |
   | -0 :warning: |  javac  |   0m 29s |  hbase-balancer generated 1 new + 17 unchanged - 0 fixed = 18 total (was 17)  |
   | +1 :green_heart: |  checkstyle  |   0m 13s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  19m 56s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   0m 44s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 11s |  The patch does not generate ASF License warnings.  |
   |  |   |  40m 21s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/9/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 1f404cee85e3 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 1e763d521f |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/9/artifact/yetus-general-check/output/diff-compile-javac-hbase-balancer.txt |
   | Max. process+thread count | 86 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/9/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  7s |  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  |   4m 12s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 13s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   0m 34s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  3s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  the patch passed  |
   | -0 :warning: |  javac  |   0m 30s |  hbase-balancer generated 1 new + 17 unchanged - 0 fixed = 18 total (was 17)  |
   | +1 :green_heart: |  checkstyle  |   0m 13s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  19m 56s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   0m 44s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 10s |  The patch does not generate ASF License warnings.  |
   |  |   |  40m 31s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/8/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 4825523144ca 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 1e763d521f |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/8/artifact/yetus-general-check/output/diff-compile-javac-hbase-balancer.txt |
   | Max. process+thread count | 86 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/8/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 26s |  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  |   3m 37s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m  4s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 17s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 43s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 13s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |  38m  5s |  hbase-balancer in the patch failed.  |
   |  |   |  64m 38s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/8/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 1a4724dbb369 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 / 1e763d521f |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/8/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-balancer.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/8/testReport/ |
   | Max. process+thread count | 354 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/8/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :confetti_ball: **+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  |   5m  5s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   9m 54s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 19s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 33s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 23s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 23s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 30s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 17s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   8m 15s |  hbase-balancer in the patch passed.  |
   |  |   |  39m 27s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/7/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 8ae85eda3cf5 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 / bb84892550 |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/7/testReport/ |
   | Max. process+thread count | 319 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/7/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 31s |  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 38s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 17s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 19s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 22s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 21s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 18s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   0m 33s |  hbase-balancer in the patch failed.  |
   |  |   |  29m 12s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 45e1becfbaff 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 / fa2d127b7f |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/2/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-balancer.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/2/testReport/ |
   | Max. process+thread count | 236 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/2/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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 #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 36s |  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  |   5m 21s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 41s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 21s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   0m 47s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 30s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 36s |  the patch passed  |
   | -0 :warning: |  javac  |   0m 36s |  hbase-balancer generated 1 new + 17 unchanged - 0 fixed = 18 total (was 17)  |
   | -0 :warning: |  checkstyle  |   0m 17s |  hbase-balancer: The patch generated 1 new + 75 unchanged - 1 fixed = 76 total (was 76)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  25m 23s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   0m 58s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 14s |  The patch does not generate ASF License warnings.  |
   |  |   |  50m  6s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux a419c5be05cd 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 / 9a324bd4d0 |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/1/artifact/yetus-general-check/output/diff-compile-javac-hbase-balancer.txt |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-balancer.txt |
   | Max. process+thread count | 96 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/1/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.12.0 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] saintstack commented on a change in pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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



##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerClusterState.java
##########
@@ -290,7 +294,9 @@ public String getRack(ServerName server) {
     }
 
     numTables = tables.size();
+    LOG.debug("number of tables = {}", numTables);

Review comment:
       nit: for next time, we capitalize log messages and if you look at other logs, there is no space around '=' when used in log messages.

##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerClusterState.java
##########
@@ -301,15 +307,26 @@ public String getRack(ServerName server) {
     for (int i = 0; i < regionIndexToServerIndex.length; i++) {
       if (regionIndexToServerIndex[i] >= 0) {
         numRegionsPerServerPerTable[regionIndexToServerIndex[i]][regionIndexToTableIndex[i]]++;
+        numRegionsPerTable[regionIndexToTableIndex[i]]++;
       }
     }
 
-    numMaxRegionsPerTable = new int[numTables];
+    // Avoid repeated computation for planning
+    meanRegionsPerTable = new double[numTables];
+    regionSkewByTable = new double[numTables];
+    maxRegionSkewByTable  = new double[numTables];
+    minRegionSkewByTable = new double[numTables];
+
+    for (int i = 0; i < numTables; i++) {
+      meanRegionsPerTable[i] = Double.valueOf(numRegionsPerTable[i]) / numServers;
+      minRegionSkewByTable[i] += DoubleArrayCost.getMinSkew(numRegionsPerTable[i], numServers);
+      maxRegionSkewByTable[i] += DoubleArrayCost.getMaxSkew(numRegionsPerTable[i], numServers);
+    }
+
     for (int[] aNumRegionsPerServerPerTable : numRegionsPerServerPerTable) {
-      for (tableIndex = 0; tableIndex < aNumRegionsPerServerPerTable.length; tableIndex++) {
-        if (aNumRegionsPerServerPerTable[tableIndex] > numMaxRegionsPerTable[tableIndex]) {
-          numMaxRegionsPerTable[tableIndex] = aNumRegionsPerServerPerTable[tableIndex];
-        }
+      for (int tableIdx = 0; tableIdx < aNumRegionsPerServerPerTable.length; tableIdx++) {
+        regionSkewByTable[tableIdx] += Math.abs(aNumRegionsPerServerPerTable[tableIdx]
+          - meanRegionsPerTable[tableIdx]);

Review comment:
       nit: style. In the code base, we usually have operator on the end of the line rather than the start as it is here... When on the end of the line, the line looks to be 'hanging' so dev will continue reading... With this style, the dev might miss the continuation. Just style.

##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/DoubleArrayCost.java
##########
@@ -106,4 +88,32 @@ private static double getSum(double[] stats) {
     }
     return total;
   }
+
+  /**
+   * Return the min skew of distribution
+   */
+  public static double getMinSkew(double total, double numServers) {

Review comment:
       nit: one way of addressing the nick comment would be to add javadoc for total that said what it was.....

##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerClusterState.java
##########
@@ -671,22 +688,21 @@ void regionMoved(int region, int oldServer, int newServer) {
     int tableIndex = regionIndexToTableIndex[region];
     if (oldServer >= 0) {
       numRegionsPerServerPerTable[oldServer][tableIndex]--;
+      // update regionSkewPerTable for the move from old server
+      regionSkewByTable[tableIndex] +=
+        Math.abs(numRegionsPerServerPerTable[oldServer][tableIndex]
+          - meanRegionsPerTable[tableIndex])
+          - Math.abs(numRegionsPerServerPerTable[oldServer][tableIndex] + 1
+          - meanRegionsPerTable[tableIndex]);
     }
     numRegionsPerServerPerTable[newServer][tableIndex]++;
 
-    // check whether this caused maxRegionsPerTable in the new Server to be updated
-    if (numRegionsPerServerPerTable[newServer][tableIndex] > numMaxRegionsPerTable[tableIndex]) {
-      numMaxRegionsPerTable[tableIndex] = numRegionsPerServerPerTable[newServer][tableIndex];
-    } else if (oldServer >= 0 && (numRegionsPerServerPerTable[oldServer][tableIndex]
-        + 1) == numMaxRegionsPerTable[tableIndex]) {
-      // recompute maxRegionsPerTable since the previous value was coming from the old server
-      numMaxRegionsPerTable[tableIndex] = 0;
-      for (int[] aNumRegionsPerServerPerTable : numRegionsPerServerPerTable) {
-        if (aNumRegionsPerServerPerTable[tableIndex] > numMaxRegionsPerTable[tableIndex]) {
-          numMaxRegionsPerTable[tableIndex] = aNumRegionsPerServerPerTable[tableIndex];
-        }
-      }
-    }
+    // update regionSkewPerTable for the move to new server
+    regionSkewByTable[tableIndex] +=
+      Math.abs(numRegionsPerServerPerTable[newServer][tableIndex]
+        - meanRegionsPerTable[tableIndex])
+        - Math.abs(numRegionsPerServerPerTable[newServer][tableIndex] - 1
+        - meanRegionsPerTable[tableIndex]);

Review comment:
       nit: hard to read. You might do local variables just to make it easier in the future rather than this long line

##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/DoubleArrayCost.java
##########
@@ -72,31 +72,13 @@ private static double computeCost(double[] stats) {
     double count = stats.length;
     double mean = total / count;
 
-    // Compute max as if all region servers had 0 and one had the sum of all costs. This must be
-    // a zero sum cost for this to make sense.
-    double max = ((count - 1) * mean) + (total - mean);
-
-    // It's possible that there aren't enough regions to go around
-    double min;
-    if (count > total) {
-      min = ((count - total) * mean) + ((1 - mean) * total);
-    } else {
-      // Some will have 1 more than everything else.
-      int numHigh = (int) (total - (Math.floor(mean) * count));
-      int numLow = (int) (count - numHigh);
-
-      min = (numHigh * (Math.ceil(mean) - mean)) + (numLow * (mean - Math.floor(mean)));
-
-    }
-    min = Math.max(0, min);

Review comment:
       Unused?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] clarax commented on a change in pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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



##########
File path: hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/DoubleArrayCost.java
##########
@@ -72,31 +72,13 @@ private static double computeCost(double[] stats) {
     double count = stats.length;
     double mean = total / count;
 
-    // Compute max as if all region servers had 0 and one had the sum of all costs. This must be
-    // a zero sum cost for this to make sense.
-    double max = ((count - 1) * mean) + (total - mean);
-
-    // It's possible that there aren't enough regions to go around
-    double min;
-    if (count > total) {
-      min = ((count - total) * mean) + ((1 - mean) * total);
-    } else {
-      // Some will have 1 more than everything else.
-      int numHigh = (int) (total - (Math.floor(mean) * count));
-      int numLow = (int) (count - numHigh);
-
-      min = (numHigh * (Math.ceil(mean) - mean)) + (numLow * (mean - Math.floor(mean)));
-
-    }
-    min = Math.max(0, min);

Review comment:
       used 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 28s |  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  |   4m  0s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 21s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 17s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 19s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 38s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 20s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 20s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   8m 11s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 17s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   7m 42s |  hbase-balancer in the patch passed.  |
   |  |   |  34m 48s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/6/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3415 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 3af058c458c7 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 / d11dc81721 |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/6/testReport/ |
   | Max. process+thread count | 354 (vs. ulimit of 30000) |
   | modules | C: hbase-balancer U: hbase-balancer |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3415/6/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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] clarax commented on pull request #3415: HBASE-25739 TableSkewCostFunction need to use aggregated deviation

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


   "values like minCostNeedBalance below are at 0.00 precision, so I think we should have an epsilon of at least 0.000 precision."
   
   Actually not really. this is aggregated deviation before scaling or divided by multiplier so it is at the precision of close to 1 or 0.1. But it is good to go lower to 0.001.


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