You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2020/04/24 23:55:33 UTC

[GitHub] [hadoop] fengnanli opened a new pull request #1977: HADOOP-17010 Add queue capacity support for FairCallQueue

fengnanli opened a new pull request #1977:
URL: https://github.com/apache/hadoop/pull/1977


   ## NOTICE
   
   Please create an issue in ASF JIRA before opening a pull request,
   and you need to set the title of the pull request which starts with
   the corresponding JIRA issue number. (e.g. HADOOP-XXXXX. Fix a typo in YYY.)
   For more details, please see https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute
   


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



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] sunchao commented on a change in pull request #1977: HADOOP-17010. Add queue capacity support for FairCallQueue

Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #1977:
URL: https://github.com/apache/hadoop/pull/1977#discussion_r416001217



##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/FairCallQueue.java
##########
@@ -101,11 +111,18 @@ public FairCallQueue(int priorityLevels, int capacity, String ns,
 
     this.queues = new ArrayList<BlockingQueue<E>>(numQueues);
     this.overflowedCalls = new ArrayList<AtomicLong>(numQueues);
-    int queueCapacity = capacity / numQueues;
-    int capacityForFirstQueue = queueCapacity + (capacity % numQueues);
+    int totalWeights = 0;
+    for (int i = 0; i < capacityWeights.length; i++) {

Review comment:
       what if user passes in negative numbers as weights? should we add a safe guard against that?

##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallQueueManager.java
##########
@@ -343,6 +347,36 @@ private static int parseNumLevels(String ns, Configuration conf) {
     return retval;
   }
 
+  /**
+   * Read the weights of capacity in callqueue and pass the value to
+   * callqueue constructions.
+   */
+  private static int[] parseCapacityWeights(
+      int priorityLevels, String ns, Configuration conf) {
+    int[] weights = conf.getInts(ns + "." +
+      CommonConfigurationKeys.IPC_CALLQUEUE_CAPACITY_WEIGHTS_KEY);
+    if (weights.length == 0) {
+      weights = getDefaultQueueCapacityWeights(priorityLevels);
+    } else if (weights.length != priorityLevels) {
+      throw new IllegalArgumentException(
+          CommonConfigurationKeys.IPC_CALLQUEUE_CAPACITY_WEIGHTS_KEY + " must "
+              + "specify " + priorityLevels + " capacity weights: one for each "
+              + "priority level");
+    }
+    return weights;
+  }
+
+  /**
+   * By default, queue capacity is the same for all priority levels.

Review comment:
       nit: add a blank line between this and the first parameter line.

##########
File path: hadoop-common-project/hadoop-common/src/site/markdown/FairCallQueue.md
##########
@@ -126,6 +126,7 @@ omitted.
 |:---- |:---- |:---- |:--- |
 | backoff.enable | General | Whether or not to enable client backoff when a queue is full. | false |
 | callqueue.impl | General | The fully qualified name of a class to use as the implementation of a call queue. Use `org.apache.hadoop.ipc.FairCallQueue` for the Fair Call Queue. | `java.util.concurrent.LinkedBlockingQueue` (FIFO queue) |
+| callqueue.capacity.weights | General | The capacity allocation weights among all subqueues. A int array whose length is equal to the `scheduler.priority.levels` is expected where each int is the relative weight out of total capacity. i.e. if a queue with capacity weight `w`, its queue capacity is `capacity * w/sum(weights)` |

Review comment:
       maybe make it explicit only positive numbers are allowed?




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



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #1977: HADOOP-17010 Add queue capacity support for FairCallQueue

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #1977:
URL: https://github.com/apache/hadoop/pull/1977#issuecomment-619289779


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  9s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | -1 :x: |  mvninstall  |  11m 13s |  root in trunk failed.  |
   | -1 :x: |  compile  |   0m 45s |  root in trunk failed.  |
   | +1 :green_heart: |  checkstyle  |   0m 45s |  trunk passed  |
   | -1 :x: |  mvnsite  |   0m 27s |  hadoop-common in trunk failed.  |
   | -1 :x: |  shadedclient  |   1m 47s |  branch has errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  1s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   3m 30s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | -1 :x: |  findbugs  |   0m 40s |  hadoop-common in trunk failed.  |
   ||| _ Patch Compile Tests _ |
   | -1 :x: |  mvninstall  |   0m 36s |  hadoop-common in the patch failed.  |
   | -1 :x: |  compile  |   0m 26s |  root in the patch failed.  |
   | -1 :x: |  javac  |   0m 26s |  root in the patch failed.  |
   | +1 :green_heart: |  checkstyle  |   0m 39s |  the patch passed  |
   | -1 :x: |  mvnsite  |   0m 24s |  hadoop-common in the patch failed.  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |   0m 21s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 48s |  the patch passed  |
   | -1 :x: |  findbugs  |   0m 16s |  hadoop-common in the patch failed.  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   0m 25s |  hadoop-common in the patch failed.  |
   | +0 :ok: |  asflicense  |   0m  8s |  ASF License check generated no output?  |
   |  |   |  23m 14s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1977 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle markdownlint |
   | uname | Linux 485f436d5dc0 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 | personality/hadoop.sh |
   | git revision | trunk / 453771f |
   | Default Java | Private Build-1.8.0_252-8u252-b09-1~16.04-b09 |
   | mvninstall | https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/1/artifact/out/branch-mvninstall-root.txt |
   | compile | https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/1/artifact/out/branch-compile-root.txt |
   | mvnsite | https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/1/artifact/out/branch-mvnsite-hadoop-common-project_hadoop-common.txt |
   | findbugs | https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/1/artifact/out/branch-findbugs-hadoop-common-project_hadoop-common.txt |
   | mvninstall | https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/1/artifact/out/patch-mvninstall-hadoop-common-project_hadoop-common.txt |
   | compile | https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/1/artifact/out/patch-compile-root.txt |
   | javac | https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/1/artifact/out/patch-compile-root.txt |
   | mvnsite | https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/1/artifact/out/patch-mvnsite-hadoop-common-project_hadoop-common.txt |
   | findbugs | https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/1/artifact/out/patch-findbugs-hadoop-common-project_hadoop-common.txt |
   | unit | https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/1/artifact/out/patch-unit-hadoop-common-project_hadoop-common.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/1/testReport/ |
   | Max. process+thread count | 94 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/1/console |
   | versions | git=2.7.4 maven=3.3.9 |
   | 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



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #1977: HADOOP-17010. Add queue capacity support for FairCallQueue

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #1977:
URL: https://github.com/apache/hadoop/pull/1977#issuecomment-620277199


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  0s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  20m  3s |  trunk passed  |
   | +1 :green_heart: |  compile  |  17m 56s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 47s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 31s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  17m 18s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  5s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   2m 13s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   2m 11s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 51s |  the patch passed  |
   | +1 :green_heart: |  compile  |  17m  5s |  the patch passed  |
   | +1 :green_heart: |  javac  |  17m  5s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 51s |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   1m 37s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  15m 30s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  5s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   2m 19s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   9m 19s |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 55s |  The patch does not generate ASF License warnings.  |
   |  |   | 112m 45s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hadoop.fs.ftp.TestFTPFileSystem |
   |   | hadoop.metrics2.source.TestJvmMetrics |
   |   | hadoop.io.compress.snappy.TestSnappyCompressorDecompressor |
   |   | hadoop.security.token.delegation.TestZKDelegationTokenSecretManager |
   |   | hadoop.io.compress.TestCompressorDecompressor |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1977 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle markdownlint |
   | uname | Linux 2bb84bc623ab 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 | personality/hadoop.sh |
   | git revision | trunk / 6ef01646ba9 |
   | Default Java | Private Build-1.8.0_252-8u252-b09-1~18.04-b09 |
   | unit | https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/2/artifact/out/patch-unit-hadoop-common-project_hadoop-common.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/2/testReport/ |
   | Max. process+thread count | 1593 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1977/2/console |
   | versions | git=2.17.1 maven=3.6.0 findbugs=3.1.0-RC1 |
   | 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



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] sunchao commented on pull request #1977: HADOOP-17010. Add queue capacity support for FairCallQueue

Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #1977:
URL: https://github.com/apache/hadoop/pull/1977#issuecomment-620903073


   Test failures do not look related. Merged. Thanks @fengnanli !


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



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org