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 2021/01/25 23:01:32 UTC
[GitHub] [hadoop] fengnanli opened a new pull request #2651: RBF: Improving Router Connection Management [WIP]
fengnanli opened a new pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651
## 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] goiri commented on a change in pull request #2651: HDFS-15757 RBF: Improving Router Connection Management [WIP]
Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#discussion_r564222047
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionContext.java
##########
@@ -42,7 +43,10 @@
private int numThreads = 0;
/** If the connection is closed. */
private boolean closed = false;
-
+ /** Last timestamp the connection was active. */
+ private long lastActiveTs = 0;
+ /** The connection's active status would expire after this window. */
+ private long activeWindow = 30 * 1000;
Review comment:
Let's add the unit and probably do: activeWindowMs = TimeUnit.SECONDS.toMillis(30)
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/FederationRPCMetrics.java
##########
@@ -208,6 +208,16 @@ public int getRpcClientNumActiveConnections() {
return rpcServer.getRPCClient().getNumActiveConnections();
}
+ @Override
+ public int getRpcClientNumIdleConnections() {
+ return rpcServer.getRPCClient().getNumIdleConnections();
Review comment:
Can we extend some test to measure this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] fengnanli commented on pull request #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
fengnanli commented on pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#issuecomment-771903717
> Thanks @fengnanli for you works here. Leave some nit comment inline.
> Sorry I do not get why the change can reduce connections here after review the changes, is it related "Be greedy here to close as many connections as possible in one shot"? It will be helpful if we add some javadocs explicitly. Thanks.
Thanks for the review @Hexiaoqiao I put the reason behind this change in the design doc in the original JIRA ticket. In short, I did synchronous connection closing + better picking connections + greedy closing connections. I have seen 50% reduce in number of connections and better ProxyTime. It will be great if you can try in your setup as well.
----------------------------------------------------------------
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] fengnanli commented on a change in pull request #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
fengnanli commented on a change in pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#discussion_r568866912
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionPool.java
##########
@@ -252,19 +252,23 @@ public synchronized void addConnection(ConnectionContext conn) {
*/
public synchronized List<ConnectionContext> removeConnections(int num) {
List<ConnectionContext> removed = new LinkedList<>();
-
- // Remove and close the last connection
- List<ConnectionContext> tmpConnections = new ArrayList<>();
- for (int i=0; i<this.connections.size(); i++) {
- ConnectionContext conn = this.connections.get(i);
- if (i < this.minSize || i < this.connections.size() - num) {
- tmpConnections.add(conn);
- } else {
- removed.add(conn);
+ if (this.connections.size() > this.minSize) {
+ int targetCount = Math.min(num, this.connections.size() - this.minSize);
Review comment:
I don't think it can negative here since the only place connections become less is in this function at the swap part with the tmpConnections. The other place where this var gets assigned is in the creation part and it can only increase the value.
----------------------------------------------------------------
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] fengnanli commented on pull request #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
fengnanli commented on pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#issuecomment-771903717
> Thanks @fengnanli for you works here. Leave some nit comment inline.
> Sorry I do not get why the change can reduce connections here after review the changes, is it related "Be greedy here to close as many connections as possible in one shot"? It will be helpful if we add some javadocs explicitly. Thanks.
Thanks for the review @Hexiaoqiao I put the reason behind this change in the design doc in the original JIRA ticket. In short, I did synchronous connection closing + better picking connections + greedy closing connections. I have seen 50% reduce in number of connections and better ProxyTime. It will be great if you can try in your setup as well.
----------------------------------------------------------------
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 #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#issuecomment-843825343
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 8s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell 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 | 36m 57s | | trunk passed |
| +1 :green_heart: | compile | 0m 39s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 0m 34s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 0m 22s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 39s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 38s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 51s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 17s | | trunk passed |
| +1 :green_heart: | shadedclient | 17m 5s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 32s | | the patch passed |
| +1 :green_heart: | compile | 0m 34s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 0m 34s | | the patch passed |
| +1 :green_heart: | compile | 0m 28s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | javac | 0m 28s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 15s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 32s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 30s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 47s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 22s | | the patch passed |
| +1 :green_heart: | shadedclient | 16m 34s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 23m 33s | | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 30s | | The patch does not generate ASF License warnings. |
| | | 107m 14s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/2651 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux cc14225ae987 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/bin/hadoop.sh |
| git revision | trunk / 95fe6e9ac2db2da87dfbb78870bd57a3f67ebc16 |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/2/testReport/ |
| Max. process+thread count | 2546 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs-rbf U: hadoop-hdfs-project/hadoop-hdfs-rbf |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/2/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT 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] fengnanli commented on a change in pull request #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
fengnanli commented on a change in pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#discussion_r634928450
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionContext.java
##########
@@ -83,30 +98,38 @@ public synchronized boolean isUsable() {
*/
public synchronized ProxyAndInfo<?> getClient() {
this.numThreads++;
+ this.lastActiveTs = Time.monotonicNow();
return this.client;
}
/**
- * Release this connection. If the connection was closed, close the proxy.
- * Otherwise, mark the connection as not used by us anymore.
+ * Release this connection.
*/
public synchronized void release() {
- if (--this.numThreads == 0 && this.closed) {
- close();
+ if (this.numThreads > 0) {
+ this.numThreads--;
}
}
/**
- * We will not use this connection anymore. If it's not being used, we close
- * it. Otherwise, we let release() do it once we are done with it.
+ * Close a connection. Only idle connections can be closed since
+ * the RPC proxy would be shut down immediately.
+ *
+ * @param force whether the connection should be closed anyway.
+ * @throws IllegalStateException when the connection is not idle
*/
- public synchronized void close() {
- this.closed = true;
- if (this.numThreads == 0) {
- Object proxy = this.client.getProxy();
- // Nobody should be using this anymore so it should close right away
- RPC.stopProxy(proxy);
+ public synchronized void close(boolean force) throws IllegalStateException {
+ if (!force && this.numThreads > 0) {
+ throw new IllegalStateException("Active connection cannot be closed");
Review comment:
Good point @DazhuangSu I made the change accordingly to replace the exception with an error log (since there is not much we can do even we catch the exception).
@Hexiaoqiao @goiri Can we consider merging this one since it is there for a long time and there is no issue from some users' experience (except for the this suggestion which will be fixed.)
--
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] goiri commented on a change in pull request #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#discussion_r568315552
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionContext.java
##########
@@ -42,7 +44,10 @@
private int numThreads = 0;
/** If the connection is closed. */
private boolean closed = false;
-
+ /** Last timestamp the connection was active. */
+ private long lastActiveTs = 0;
+ /** The connection's active status would expire after this window. */
+ private long activeWindow = TimeUnit.SECONDS.toMillis(30);
Review comment:
I agree. and called ACTIVE_WINDOW_TIME
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionContext.java
##########
@@ -57,6 +62,17 @@ public synchronized boolean isActive() {
return this.numThreads > 0;
}
+ /**
+ * Check if the connection is/was active recently.
+ *
+ * @return True if the connection is active or
+ * was active in the past period of time.
+ */
+ public synchronized boolean isActiveRecently() {
+ return isActive() ||
Review comment:
If we do the timestamp based, do we even need to check for the isActive() or the timestamp comparisson is enoug?
----------------------------------------------------------------
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] goiri merged pull request #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
goiri merged pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651
--
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 #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#issuecomment-771998442
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 6m 45s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | | 0m 0s | [test4tests](test4tests) | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 40m 40s | | trunk passed |
| +1 :green_heart: | compile | 0m 50s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | compile | 0m 46s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +1 :green_heart: | checkstyle | 0m 34s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 53s | | trunk passed |
| +1 :green_heart: | shadedclient | 19m 42s | | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 48s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 1m 6s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +0 :ok: | spotbugs | 1m 48s | | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 1m 42s | | trunk passed |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 44s | | the patch passed |
| +1 :green_heart: | compile | 0m 45s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javac | 0m 45s | | the patch passed |
| +1 :green_heart: | compile | 0m 36s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +1 :green_heart: | javac | 0m 36s | | the patch passed |
| +1 :green_heart: | checkstyle | 0m 17s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 36s | | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | | The patch has no whitespace issues. |
| +1 :green_heart: | shadedclient | 17m 1s | | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 0m 57s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +1 :green_heart: | findbugs | 1m 36s | | the patch passed |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 20m 1s | | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 33s | | The patch does not generate ASF License warnings. |
| | | 120m 39s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/6/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/2651 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux b7d09608d928 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/bin/hadoop.sh |
| git revision | trunk / f37bf651993 |
| Default Java | Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/6/testReport/ |
| Max. process+thread count | 2389 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs-rbf U: hadoop-hdfs-project/hadoop-hdfs-rbf |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/6/console |
| versions | git=2.25.1 maven=3.6.3 findbugs=4.0.6 |
| Powered by | Apache Yetus 0.13.0-SNAPSHOT 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 #2651: HDFS-15757 RBF: Improving Router Connection Management [WIP]
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#issuecomment-767205486
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 34s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | | 0m 0s | [test4tests](test4tests) | The patch appears to include 3 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 36m 27s | | trunk passed |
| +1 :green_heart: | compile | 0m 43s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 |
| +1 :green_heart: | compile | 0m 39s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| +1 :green_heart: | checkstyle | 0m 27s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 47s | | trunk passed |
| +1 :green_heart: | shadedclient | 18m 36s | | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 46s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 |
| +1 :green_heart: | javadoc | 1m 0s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| +0 :ok: | spotbugs | 1m 15s | | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 1m 14s | | trunk passed |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 34s | | the patch passed |
| +1 :green_heart: | compile | 0m 33s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 |
| +1 :green_heart: | javac | 0m 33s | | the patch passed |
| +1 :green_heart: | compile | 0m 30s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| +1 :green_heart: | javac | 0m 30s | | the patch passed |
| +1 :green_heart: | checkstyle | 0m 19s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 33s | | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | | The patch has no whitespace issues. |
| +1 :green_heart: | shadedclient | 14m 46s | | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 34s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 |
| +1 :green_heart: | javadoc | 0m 53s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| +1 :green_heart: | findbugs | 1m 18s | | the patch passed |
|||| _ Other Tests _ |
| -1 :x: | unit | 16m 33s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/1/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 35s | | The patch does not generate ASF License warnings. |
| | | 101m 43s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.federation.router.TestConnectionManager |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/2651 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 31935c7d2f66 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 06a5d3437f6 |
| Default Java | Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/1/testReport/ |
| Max. process+thread count | 2464 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs-rbf U: hadoop-hdfs-project/hadoop-hdfs-rbf |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/1/console |
| versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
| Powered by | Apache Yetus 0.13.0-SNAPSHOT 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] Hexiaoqiao commented on pull request #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
Hexiaoqiao commented on pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#issuecomment-843815151
Thanks @fengnanli for your work.
It is safe to checkin for me now. @goiri What do you think about?
--
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 #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#issuecomment-770142137
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 2m 1s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | | 0m 0s | [test4tests](test4tests) | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 39m 7s | | trunk passed |
| +1 :green_heart: | compile | 0m 56s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | compile | 0m 40s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +1 :green_heart: | checkstyle | 0m 29s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 44s | | trunk passed |
| +1 :green_heart: | shadedclient | 19m 17s | | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 43s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 0m 57s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +0 :ok: | spotbugs | 1m 19s | | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 1m 16s | | trunk passed |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 34s | | the patch passed |
| +1 :green_heart: | compile | 0m 36s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javac | 0m 36s | | the patch passed |
| +1 :green_heart: | compile | 0m 33s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +1 :green_heart: | javac | 0m 33s | | the patch passed |
| +1 :green_heart: | checkstyle | 0m 18s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 34s | | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | | The patch has no whitespace issues. |
| +1 :green_heart: | shadedclient | 17m 38s | | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 33s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 0m 50s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +1 :green_heart: | findbugs | 1m 16s | | the patch passed |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 17m 12s | | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 30s | | The patch does not generate ASF License warnings. |
| | | 109m 42s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/2651 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux e92d69011a59 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/bin/hadoop.sh |
| git revision | trunk / ad483fd66e8 |
| Default Java | Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/4/testReport/ |
| Max. process+thread count | 2609 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs-rbf U: hadoop-hdfs-project/hadoop-hdfs-rbf |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/4/console |
| versions | git=2.25.1 maven=3.6.3 findbugs=4.0.6 |
| Powered by | Apache Yetus 0.13.0-SNAPSHOT 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] fengnanli commented on a change in pull request #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
fengnanli commented on a change in pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#discussion_r634928450
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionContext.java
##########
@@ -83,30 +98,38 @@ public synchronized boolean isUsable() {
*/
public synchronized ProxyAndInfo<?> getClient() {
this.numThreads++;
+ this.lastActiveTs = Time.monotonicNow();
return this.client;
}
/**
- * Release this connection. If the connection was closed, close the proxy.
- * Otherwise, mark the connection as not used by us anymore.
+ * Release this connection.
*/
public synchronized void release() {
- if (--this.numThreads == 0 && this.closed) {
- close();
+ if (this.numThreads > 0) {
+ this.numThreads--;
}
}
/**
- * We will not use this connection anymore. If it's not being used, we close
- * it. Otherwise, we let release() do it once we are done with it.
+ * Close a connection. Only idle connections can be closed since
+ * the RPC proxy would be shut down immediately.
+ *
+ * @param force whether the connection should be closed anyway.
+ * @throws IllegalStateException when the connection is not idle
*/
- public synchronized void close() {
- this.closed = true;
- if (this.numThreads == 0) {
- Object proxy = this.client.getProxy();
- // Nobody should be using this anymore so it should close right away
- RPC.stopProxy(proxy);
+ public synchronized void close(boolean force) throws IllegalStateException {
+ if (!force && this.numThreads > 0) {
+ throw new IllegalStateException("Active connection cannot be closed");
Review comment:
Good point @DazhuangSu I made the change according to replace the exception with an error log (since there is not much we can do even we catch the exception).
@Hexiaoqiao @goiri Can we consider merging this one since it is there for a long time and there is no issue from some users' experience (except for the this suggestion which will be fixed.)
--
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] DazhuangSu commented on a change in pull request #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
DazhuangSu commented on a change in pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#discussion_r628711047
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionContext.java
##########
@@ -83,30 +98,38 @@ public synchronized boolean isUsable() {
*/
public synchronized ProxyAndInfo<?> getClient() {
this.numThreads++;
+ this.lastActiveTs = Time.monotonicNow();
return this.client;
}
/**
- * Release this connection. If the connection was closed, close the proxy.
- * Otherwise, mark the connection as not used by us anymore.
+ * Release this connection.
*/
public synchronized void release() {
- if (--this.numThreads == 0 && this.closed) {
- close();
+ if (this.numThreads > 0) {
+ this.numThreads--;
}
}
/**
- * We will not use this connection anymore. If it's not being used, we close
- * it. Otherwise, we let release() do it once we are done with it.
+ * Close a connection. Only idle connections can be closed since
+ * the RPC proxy would be shut down immediately.
+ *
+ * @param force whether the connection should be closed anyway.
+ * @throws IllegalStateException when the connection is not idle
*/
- public synchronized void close() {
- this.closed = true;
- if (this.numThreads == 0) {
- Object proxy = this.client.getProxy();
- // Nobody should be using this anymore so it should close right away
- RPC.stopProxy(proxy);
+ public synchronized void close(boolean force) throws IllegalStateException {
+ if (!force && this.numThreads > 0) {
+ throw new IllegalStateException("Active connection cannot be closed");
Review comment:
@fengnanli @Hexiaoqiao
we shouldn't throw a RuntimeException here.
whenever CleanupTask run into this , throwing a runtime exception will cause the threadpool(which is used to close idle connection ) in ConnectionManager to hung.
And no more CleanupTask will be scheduled.
--
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] goiri commented on a change in pull request #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#discussion_r568315959
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionContext.java
##########
@@ -57,6 +62,17 @@ public synchronized boolean isActive() {
return this.numThreads > 0;
}
+ /**
+ * Check if the connection is/was active recently.
+ *
+ * @return True if the connection is active or
+ * was active in the past period of time.
+ */
+ public synchronized boolean isActiveRecently() {
+ return isActive() ||
Review comment:
If we do the timestamp based, do we even need to check for the isActive() or the timestamp comparisson is enoug?
----------------------------------------------------------------
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 #2651: HDFS-15757 RBF: Improving Router Connection Management [WIP]
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#issuecomment-767205486
----------------------------------------------------------------
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 #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#issuecomment-771379941
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 11s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | | 0m 0s | [test4tests](test4tests) | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 34m 59s | | trunk passed |
| +1 :green_heart: | compile | 0m 39s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | compile | 0m 33s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +1 :green_heart: | checkstyle | 0m 24s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 38s | | trunk passed |
| +1 :green_heart: | shadedclient | 16m 50s | | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 0m 53s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +0 :ok: | spotbugs | 1m 15s | | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 1m 13s | | trunk passed |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 31s | | the patch passed |
| +1 :green_heart: | compile | 0m 35s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javac | 0m 35s | | the patch passed |
| +1 :green_heart: | compile | 0m 27s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +1 :green_heart: | javac | 0m 27s | | the patch passed |
| +1 :green_heart: | checkstyle | 0m 15s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 31s | | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | | The patch has no whitespace issues. |
| +1 :green_heart: | shadedclient | 15m 11s | | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 33s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 0m 49s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +1 :green_heart: | findbugs | 1m 16s | | the patch passed |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 17m 25s | | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 29s | | The patch does not generate ASF License warnings. |
| | | 98m 47s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/2651 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 9f8acd464a83 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/bin/hadoop.sh |
| git revision | trunk / 21a3fc3d2d7 |
| Default Java | Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/5/testReport/ |
| Max. process+thread count | 2396 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs-rbf U: hadoop-hdfs-project/hadoop-hdfs-rbf |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/5/console |
| versions | git=2.25.1 maven=3.6.3 findbugs=4.0.6 |
| Powered by | Apache Yetus 0.13.0-SNAPSHOT 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 #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#issuecomment-771379941
----------------------------------------------------------------
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] goiri commented on a change in pull request #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#discussion_r568315552
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionContext.java
##########
@@ -42,7 +44,10 @@
private int numThreads = 0;
/** If the connection is closed. */
private boolean closed = false;
-
+ /** Last timestamp the connection was active. */
+ private long lastActiveTs = 0;
+ /** The connection's active status would expire after this window. */
+ private long activeWindow = TimeUnit.SECONDS.toMillis(30);
Review comment:
I agree. and called ACTIVE_WINDOW_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
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] fengnanli commented on pull request #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
fengnanli commented on pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#issuecomment-770130994
@goiri Added tests and addressed some comments.
----------------------------------------------------------------
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 #2651: HDFS-15757 RBF: Improving Router Connection Management [WIP]
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#issuecomment-767218309
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 30s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 33m 36s | | trunk passed |
| +1 :green_heart: | compile | 0m 41s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 |
| +1 :green_heart: | compile | 0m 35s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| +1 :green_heart: | checkstyle | 0m 28s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 41s | | trunk passed |
| +1 :green_heart: | shadedclient | 18m 19s | | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 49s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 |
| +1 :green_heart: | javadoc | 1m 2s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| +0 :ok: | spotbugs | 1m 33s | | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 1m 30s | | trunk passed |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 39s | | the patch passed |
| +1 :green_heart: | compile | 0m 41s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 |
| +1 :green_heart: | javac | 0m 41s | | the patch passed |
| +1 :green_heart: | compile | 0m 33s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| +1 :green_heart: | javac | 0m 33s | | the patch passed |
| +1 :green_heart: | checkstyle | 0m 20s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 36s | | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | | The patch has no whitespace issues. |
| +1 :green_heart: | shadedclient | 19m 25s | | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 |
| +1 :green_heart: | javadoc | 0m 55s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| +1 :green_heart: | findbugs | 1m 37s | | the patch passed |
|||| _ Other Tests _ |
| -1 :x: | unit | 45m 45s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/3/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 33s | | The patch does not generate ASF License warnings. |
| | | 133m 12s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.fs.contract.router.web.TestRouterWebHDFSContractMkdir |
| | hadoop.fs.contract.router.TestRouterHDFSContractRenameSecure |
| | hadoop.fs.contract.router.TestRouterHDFSContractSeek |
| | hadoop.fs.contract.router.web.TestRouterWebHDFSContractOpen |
| | hadoop.fs.contract.router.web.TestRouterWebHDFSContractConcat |
| | hadoop.hdfs.server.federation.router.TestConnectionManager |
| | hadoop.fs.contract.router.web.TestRouterWebHDFSContractCreate |
| | hadoop.fs.contract.router.web.TestRouterWebHDFSContractDelete |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/2651 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux c60e675a0121 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 06a5d3437f6 |
| Default Java | Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/3/testReport/ |
| Max. process+thread count | 2335 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs-rbf U: hadoop-hdfs-project/hadoop-hdfs-rbf |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/3/console |
| versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
| Powered by | Apache Yetus 0.13.0-SNAPSHOT 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] Hexiaoqiao commented on a change in pull request #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
Hexiaoqiao commented on a change in pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#discussion_r567408154
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionContext.java
##########
@@ -42,7 +44,10 @@
private int numThreads = 0;
/** If the connection is closed. */
private boolean closed = false;
-
+ /** Last timestamp the connection was active. */
+ private long lastActiveTs = 0;
+ /** The connection's active status would expire after this window. */
+ private long activeWindow = TimeUnit.SECONDS.toMillis(30);
Review comment:
`activeWindow` should be final static constant here?
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionPool.java
##########
@@ -252,19 +252,23 @@ public synchronized void addConnection(ConnectionContext conn) {
*/
public synchronized List<ConnectionContext> removeConnections(int num) {
List<ConnectionContext> removed = new LinkedList<>();
-
- // Remove and close the last connection
- List<ConnectionContext> tmpConnections = new ArrayList<>();
- for (int i=0; i<this.connections.size(); i++) {
- ConnectionContext conn = this.connections.get(i);
- if (i < this.minSize || i < this.connections.size() - num) {
- tmpConnections.add(conn);
- } else {
- removed.add(conn);
+ if (this.connections.size() > this.minSize) {
+ int targetCount = Math.min(num, this.connections.size() - this.minSize);
Review comment:
IIRC, `connections` is not thread safe, so is it possible `targetCount` is negative here？
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionPool.java
##########
@@ -252,19 +252,23 @@ public synchronized void addConnection(ConnectionContext conn) {
*/
public synchronized List<ConnectionContext> removeConnections(int num) {
List<ConnectionContext> removed = new LinkedList<>();
-
- // Remove and close the last connection
- List<ConnectionContext> tmpConnections = new ArrayList<>();
- for (int i=0; i<this.connections.size(); i++) {
- ConnectionContext conn = this.connections.get(i);
- if (i < this.minSize || i < this.connections.size() - num) {
- tmpConnections.add(conn);
- } else {
- removed.add(conn);
+ if (this.connections.size() > this.minSize) {
+ int targetCount = Math.min(num, this.connections.size() - this.minSize);
+ // Remove and close targetCount of connections
+ List<ConnectionContext> tmpConnections = new ArrayList<>();
+ for (int i=0; i<this.connections.size(); i++) {
Review comment:
nit: code style.
----------------------------------------------------------------
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] fengnanli commented on a change in pull request #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
fengnanli commented on a change in pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#discussion_r568868167
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionContext.java
##########
@@ -57,6 +62,17 @@ public synchronized boolean isActive() {
return this.numThreads > 0;
}
+ /**
+ * Check if the connection is/was active recently.
+ *
+ * @return True if the connection is active or
+ * was active in the past period of time.
+ */
+ public synchronized boolean isActiveRecently() {
+ return isActive() ||
Review comment:
That can removed since the timewindow calculation covers the active case. Updated.
----------------------------------------------------------------
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] fengnanli commented on a change in pull request #2651: HDFS-15757 RBF: Improving Router Connection Management
Posted by GitBox <gi...@apache.org>.
fengnanli commented on a change in pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#discussion_r568866912
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionPool.java
##########
@@ -252,19 +252,23 @@ public synchronized void addConnection(ConnectionContext conn) {
*/
public synchronized List<ConnectionContext> removeConnections(int num) {
List<ConnectionContext> removed = new LinkedList<>();
-
- // Remove and close the last connection
- List<ConnectionContext> tmpConnections = new ArrayList<>();
- for (int i=0; i<this.connections.size(); i++) {
- ConnectionContext conn = this.connections.get(i);
- if (i < this.minSize || i < this.connections.size() - num) {
- tmpConnections.add(conn);
- } else {
- removed.add(conn);
+ if (this.connections.size() > this.minSize) {
+ int targetCount = Math.min(num, this.connections.size() - this.minSize);
Review comment:
I don't think it can negative here since the only place connections become less is in this function at the swap part with the tmpConnections. The other place where this var gets assigned is in the creation part and it can only increase the value.
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionContext.java
##########
@@ -57,6 +62,17 @@ public synchronized boolean isActive() {
return this.numThreads > 0;
}
+ /**
+ * Check if the connection is/was active recently.
+ *
+ * @return True if the connection is active or
+ * was active in the past period of time.
+ */
+ public synchronized boolean isActiveRecently() {
+ return isActive() ||
Review comment:
That can removed since the timewindow calculation covers the active case. Updated.
----------------------------------------------------------------
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 #2651: HDFS-15757 RBF: Improving Router Connection Management [WIP]
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2651:
URL: https://github.com/apache/hadoop/pull/2651#issuecomment-767213511
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 50s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 41m 35s | | trunk passed |
| +1 :green_heart: | compile | 0m 45s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 |
| +1 :green_heart: | compile | 0m 46s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| +1 :green_heart: | checkstyle | 0m 30s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 50s | | trunk passed |
| +1 :green_heart: | shadedclient | 21m 45s | | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 49s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 |
| +1 :green_heart: | javadoc | 0m 56s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| +0 :ok: | spotbugs | 1m 32s | | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 1m 29s | | trunk passed |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 40s | | the patch passed |
| +1 :green_heart: | compile | 0m 42s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 |
| +1 :green_heart: | javac | 0m 42s | | the patch passed |
| +1 :green_heart: | compile | 0m 37s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| +1 :green_heart: | javac | 0m 37s | | the patch passed |
| +1 :green_heart: | checkstyle | 0m 21s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 41s | | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | | The patch has no whitespace issues. |
| +1 :green_heart: | shadedclient | 19m 39s | | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 0m 44s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 |
| +1 :green_heart: | javadoc | 1m 7s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| +1 :green_heart: | findbugs | 1m 52s | | the patch passed |
|||| _ Other Tests _ |
| -1 :x: | unit | 18m 58s | [/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/2/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt) | hadoop-hdfs-rbf in the patch passed. |
| +1 :green_heart: | asflicense | 0m 32s | | The patch does not generate ASF License warnings. |
| | | 120m 34s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.federation.router.TestConnectionManager |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/2651 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 214833bb9ee4 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 06a5d3437f6 |
| Default Java | Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/2/testReport/ |
| Max. process+thread count | 2060 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs-rbf U: hadoop-hdfs-project/hadoop-hdfs-rbf |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2651/2/console |
| versions | git=2.17.1 maven=3.6.0 findbugs=4.0.6 |
| Powered by | Apache Yetus 0.13.0-SNAPSHOT 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