[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-09 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/3625


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-09 Thread rxin
Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-66392255
  
I'm merging this one in master & branch-1.2.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65886768
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24207/
Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65886767
  
  [Test build #24207 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24207/consoleFull)
 for   PR 3625 at commit 
[`ad4241a`](https://github.com/apache/spark/commit/ad4241ad8e6f353e5be872e20f6c4261180c648a).
 * This patch **passes all tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65885551
  
  [Test build #24206 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24206/consoleFull)
 for   PR 3625 at commit 
[`ad4241a`](https://github.com/apache/spark/commit/ad4241ad8e6f353e5be872e20f6c4261180c648a).
 * This patch **passes all tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65885553
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24206/
Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65885123
  
  [Test build #24207 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24207/consoleFull)
 for   PR 3625 at commit 
[`ad4241a`](https://github.com/apache/spark/commit/ad4241ad8e6f353e5be872e20f6c4261180c648a).
 * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread rxin
Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65885045
  
Jenkins, retest this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread aarondav
Github user aarondav commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65884482
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65883784
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24205/
Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65883623
  
  [Test build #24206 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24206/consoleFull)
 for   PR 3625 at commit 
[`ad4241a`](https://github.com/apache/spark/commit/ad4241ad8e6f353e5be872e20f6c4261180c648a).
 * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread aarondav
Github user aarondav commented on a diff in the pull request:

https://github.com/apache/spark/pull/3625#discussion_r21413733
  
--- Diff: 
network/common/src/test/java/org/apache/spark/network/TransportClientFactorySuite.java
 ---
@@ -57,16 +62,113 @@ public void tearDown() {
 JavaUtils.closeQuietly(server2);
   }
 
+  /**
+   * Request a bunch of clients to a single server to test
+   * we create up to maxConnections of clients.
+   */
+  private void testClientReuse(final int maxConnections) throws 
IOException {
+TransportConf conf = new TransportConf(new ConfigProvider() {
+  @Override
+  public String get(String name) {
+if (name.equals("spark.shuffle.io.numConnectionsPerPeer")) {
+  return Integer.toString(maxConnections);
+} else {
+  throw new NoSuchElementException();
+}
+  }
+});
+
+RpcHandler rpcHandler = new NoOpRpcHandler();
+TransportContext context = new TransportContext(conf, rpcHandler);
+TransportClientFactory factory = context.createClientFactory();
+HashSet clients = new HashSet();
+for (int i = 0; i < maxConnections * 10; i++) {
+  TransportClient client = 
factory.createClient(TestUtils.getLocalHost(), server1.getPort());
+  assert(client.isActive());
+  clients.add(client);
+}
+
+assert(clients.size() == maxConnections);
--- End diff --

seems like we should close the clients afterwards


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread aarondav
Github user aarondav commented on a diff in the pull request:

https://github.com/apache/spark/pull/3625#discussion_r21413717
  
--- Diff: 
network/common/src/main/java/org/apache/spark/network/client/TransportClientFactory.java
 ---
@@ -143,43 +190,41 @@ public void initChannel(SocketChannel ch) {
 assert client != null : "Channel future completed successfully with 
null client";
 
 // Execute any client bootstraps synchronously before marking the 
Client as successful.
-long preBootstrap = System.currentTimeMillis();
+long preBootstrap = System.nanoTime();
 logger.debug("Connection to {} successful, running bootstraps...", 
address);
 try {
   for (TransportClientBootstrap clientBootstrap : clientBootstraps) {
 clientBootstrap.doBootstrap(client);
   }
 } catch (Exception e) { // catch non-RuntimeExceptions too as 
bootstrap may be written in Scala
-  long bootstrapTime = System.currentTimeMillis() - preBootstrap;
-  logger.error("Exception while bootstrapping client after " + 
bootstrapTime + " ms", e);
+  long bootstrapTimeMs = (System.nanoTime() - preBootstrap) / 100;
+  logger.error("Exception while bootstrapping client after " + 
bootstrapTimeMs + " ms", e);
   client.close();
   throw Throwables.propagate(e);
 }
-long postBootstrap = System.currentTimeMillis();
-
-// Successful connection & bootstrap -- in the event that two threads 
raced to create a client,
-// use the first one that was put into the connectionPool and close 
the one we made here.
-TransportClient oldClient = connectionPool.putIfAbsent(address, 
client);
-if (oldClient == null) {
-  logger.debug("Successfully created connection to {} after {} ms ({} 
ms spent in bootstraps)",
-address, postBootstrap - preConnect, postBootstrap - preBootstrap);
-  return client;
-} else {
-  logger.debug("Two clients were created concurrently after {} ms, 
second will be disposed.",
-postBootstrap - preConnect);
-  client.close();
-  return oldClient;
-}
+long postBootstrap = System.nanoTime();
+
+logger.debug("Successfully created connection to {} after {} ms ({} ms 
spent in bootstraps)",
+  address, (postBootstrap - preConnect) / 100, (postBootstrap - 
preBootstrap) / 100);
+
+return client;
   }
 
   /** Close all connections in the connection pool, and shutdown the 
worker thread pool. */
   @Override
   public void close() {
-for (TransportClient client : connectionPool.values()) {
-  try {
-client.close();
-  } catch (RuntimeException e) {
-logger.warn("Ignoring exception during close", e);
+// Go through all clients and close them if they are active.
+for (ClientPool clientPool : connectionPool.values()) {
+  for (int i = 0; i < clientPool.clients.length; i++) {
+TransportClient client = clientPool.clients[i];
+if (client != null) {
+  clientPool.clients[i] = null;
+  try {
+client.close();
--- End diff --

Q: Can we use JavaUtils.closeQuietly(client) here?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65880355
  
  [Test build #24200 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24200/consoleFull)
 for   PR 3625 at commit 
[`0fefabb`](https://github.com/apache/spark/commit/0fefabb9d63dec4ad2175338ee7d42ad622a9536).
 * This patch **passes all tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65880366
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24200/
Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65879737
  
  [Test build #24199 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24199/consoleFull)
 for   PR 3625 at commit 
[`41dfcb2`](https://github.com/apache/spark/commit/41dfcb260e71ecc5d497d64c80f976f350f59be5).
 * This patch **passes all tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65879741
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24199/
Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65875429
  
  [Test build #24200 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24200/consoleFull)
 for   PR 3625 at commit 
[`0fefabb`](https://github.com/apache/spark/commit/0fefabb9d63dec4ad2175338ee7d42ad622a9536).
 * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65874408
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24198/
Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65874403
  
  [Test build #24198 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24198/consoleFull)
 for   PR 3625 at commit 
[`9076b4a`](https://github.com/apache/spark/commit/9076b4a0f9e648ae5451795c3d024d491accd6e0).
 * This patch **fails Spark unit tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...

2014-12-05 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3625#issuecomment-65874329
  
  [Test build #24199 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24199/consoleFull)
 for   PR 3625 at commit 
[`41dfcb2`](https://github.com/apache/spark/commit/41dfcb260e71ecc5d497d64c80f976f350f59be5).
 * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org