[GitHub] spark pull request: [SPARK-4740] Create multiple concurrent connec...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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