Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-52385166
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
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-52385196
[QA tests have
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18662/consoleFull)
for PR 1632 at commit
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-52385237
Pending Jenkins, this looks good to me. Committers, feel free to merge (or
I'll do it).
---
If your project is set up for it, you can reply to this email and have
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16323402
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -61,17 +62,17 @@ private[spark] class ConnectionManager(
var
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16323399
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -652,19 +654,21 @@ private[spark] class ConnectionManager(
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-52385383
Actually, I retract my earlier LGTM; this needs a bit of user-facing
configuration documentation and I think there's a corner-case bug in how we
handle late-arriving
Github user sarutak commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16323483
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -652,19 +654,21 @@ private[spark] class ConnectionManager(
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16323500
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -652,19 +654,21 @@ private[spark] class ConnectionManager(
Github user sarutak commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16323512
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -652,19 +654,21 @@ private[spark] class ConnectionManager(
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-52385869
[QA tests have
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18662/consoleFull)
for PR 1632 at commit
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16323524
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -652,19 +654,21 @@ private[spark] class ConnectionManager(
Github user sarutak commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16323528
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -652,19 +654,21 @@ private[spark] class ConnectionManager(
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-52386455
[QA tests have
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18669/consoleFull)
for PR 1632 at commit
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-52406146
I've merged this to `master` and `branch-1.1`. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/1632
---
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
Github user sarutak commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16282025
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -836,9 +845,14 @@ private[spark] class ConnectionManager(
def
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-52279573
[QA tests have
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18600/consoleFull)
for PR 1632 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-52280997
[QA tests have
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18605/consoleFull)
for PR 1632 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-52343198
[QA tests have
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18623/consoleFull)
for PR 1632 at commit
Github user sarutak commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-52347790
@JoshRosen Exactly, thanks.
---
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
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-52348963
[QA tests have
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18623/consoleFull)
for PR 1632 at commit
Github user loveconan1988 commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16153410
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -652,19 +655,25 @@ private[spark] class ConnectionManager(
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-51825773
Jenkins, this is ok to test. Jenkins, test this please.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-51861681
Jenkins, test 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
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16091154
--- Diff:
core/src/test/scala/org/apache/spark/network/ConnectionManagerSuite.scala ---
@@ -255,5 +260,42 @@ class ConnectionManagerSuite extends FunSuite
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-51861905
QA tests have started for PR 1632. This patch merges cleanly. brView
progress:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18346/consoleFull
---
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16091329
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -134,6 +136,7 @@ private[spark] class ConnectionManager(
// to
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16091409
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -836,9 +845,14 @@ private[spark] class ConnectionManager(
def
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16091512
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -836,9 +845,14 @@ private[spark] class ConnectionManager(
def
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-51863140
This approach seems very complicated, race-prone, and hard to understand.
`Await.ready` and `Await.result` already support timeouts, so why not just
add
Github user shivaram commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-51864387
Another way to solve this is to change the BasicBlockFetcher to use `poll`
with a timeout in LinkedBlockingQueue [1]
[1]
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-51865034
QA results for PR 1632:br- This patch PASSES unit tests.br- This patch
merges cleanlybr- This patch adds no public classesbrbrFor more
information see test
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-51866549
@shivaram That's a really good suggestion. I'll try to write a failing
unit test that directly uses BasicBlockFetcherIterator so that we can test your
approach.
---
Github user sarutak commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-51867662
Hi @shivaram , @JoshRosen
At first, I have an idea to use poll. I thought it's the easy way.
But, if we use poll and catch TimeoutException, I think,
Github user sarutak commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-51867803
The reason why I din't use Await.ready and Await.result is because those
are blocking method. Current way which use onComplete callback is non-blocking.
---
If your
Github user sarutak commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-51868341
O.K. I'll try to resolve using poll somehow.
---
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
Github user witgo commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-51868710
I think the current solution is better. `LinkedBlockingQueue .poll` will
bring a lot of problems.
---
If your project is set up for it, you can reply to this email and
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16094928
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -836,9 +845,14 @@ private[spark] class ConnectionManager(
def
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16094973
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -836,9 +845,14 @@ private[spark] class ConnectionManager(
def
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-51872227
@sarutak I left updates on a couple of my earlier comments. This solution
can work and I have a few suggestions for minor cleanup (e.g. re-using a Timer).
---
If
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16095159
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -652,19 +655,25 @@ private[spark] class ConnectionManager(
Github user sarutak commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-51872672
@JoshRosen Thanks!
I'll try it.
---
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
Github user witgo commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16095403
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -72,6 +73,7 @@ private[spark] class ConnectionManager(
//
Github user sarutak commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16095425
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -72,6 +73,7 @@ private[spark] class ConnectionManager(
//
Github user shivaram commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-51873138
@sarutak You are right that using poll wouldn't clear up the internal state
in ConnectionManager. I think @JoshRosen 's idea of using a shared timer pool
or re-using
Github user witgo commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r16095492
--- Diff:
core/src/main/scala/org/apache/spark/network/ConnectionManager.scala ---
@@ -22,6 +22,7 @@ import java.nio._
import java.nio.channels._
Github user sarutak commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-51709077
In #1758 @JoshRosen fixed ConnectionManager to handle the case remote
executor return error message.
But, the case remote executor hangs up is not handled so if
Github user lianhuiwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r15573389
--- Diff:
core/src/main/scala/org/apache/spark/storage/BlockFetcherIterator.scala ---
@@ -117,31 +121,45 @@ object BlockFetcherIterator {
})
GitHub user sarutak opened a pull request:
https://github.com/apache/spark/pull/1632
[SPARK-2677] BasicBlockFetchIterator#next can wait forever
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sarutak/spark SPARK-2677
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1632#issuecomment-50440763
Can one of the admins verify this patch?
---
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
Github user witgo commented on a diff in the pull request:
https://github.com/apache/spark/pull/1632#discussion_r15508224
--- Diff:
core/src/main/scala/org/apache/spark/storage/BlockFetcherIterator.scala ---
@@ -117,31 +121,45 @@ object BlockFetcherIterator {
})
51 matches
Mail list logo
