You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by nezihyigitbasi <gi...@git.apache.org> on 2016/02/17 18:47:57 UTC
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
GitHub user nezihyigitbasi opened a pull request:
https://github.com/apache/spark/pull/11241
[SPARK-13328][Core]: Poor read performance for broadcast variables with dynamic resource allocation
When dynamic resource allocation is enabled fetching broadcast variables from removed executors were causing job failures and SPARK-9591 fixed this problem by trying all locations of a block before giving up. However, the locations of a block is retrieved only once from the driver in this process and the locations in this list can be stale due to dynamic resource allocation. This situation gets worse when running on a large cluster as the size of this location list can be in the order of several hundreds out of which there may be tens of stale entries. What we have observed is with the default settings of 3 max retries and 5s between retries (that's 15s per location) the time it takes to read a broadcast variable can be as high as ~17m (70 failed attempts * 15s/attempt)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nezihyigitbasi/spark SPARK-13328
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/11241.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #11241
----
commit 45bdec651a2d15ad97638a676675c1697ddade09
Author: Nezih Yigitbasi <ny...@netflix.com>
Date: 2016-02-17T17:39:06Z
Support refreshing block locations during reads
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187412639
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51687/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-191770201
Sorry but I disagree on this limit not being configurable. Depending on how big your job, cluster, and broadcast are a user may want to set this differently. I think we should make this configurable, we can leave it as an undocumented internal config for now but I would like an out if my users start hitting this. @andrewor14 thoughts?
Note I recently ran into this with dynamic allocation and it took forever for those tasks to fail. I'm in process of testing this but haven't run into that condition again yet.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187401871
**[Test build #51686 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51686/consoleFull)** for PR 11241 at commit [`e444072`](https://github.com/apache/spark/commit/e4440727b32a7fd89ee8067a78814db60f6e486e).
* This patch **fails Scala style 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195075443
@andrewor14 @tgravescs @squito guys I believe this is ready to get in. Do you have any other comments?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195113990
**[Test build #52871 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52871/consoleFull)** for PR 11241 at commit [`0875b24`](https://github.com/apache/spark/commit/0875b243ad2a17448bab742e8507818e49932a25).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195154174
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/52871/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187398698
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195105166
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/52869/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55740336
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -570,28 +576,53 @@ private[spark] class BlockManager(
preferredLocs ++ otherLocs
}
- private def doGetRemote(blockId: BlockId, asBlockResult: Boolean): Option[Any] = {
+ /**
+ * VisibleForTesting
+ */
--- End diff --
(actually we don't need to expose this, see my other comment)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55606187
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -570,28 +577,49 @@ private[spark] class BlockManager(
preferredLocs ++ otherLocs
}
- private def doGetRemote(blockId: BlockId, asBlockResult: Boolean): Option[Any] = {
+ /**
+ * VisibleForTesting
+ */
+ private[spark] def doGetRemote(blockId: BlockId, blockTransferService: BlockTransferService,
+ asBlockResult: Boolean): Option[Any] = {
require(blockId != null, "BlockId is null")
+ var runningFailureCount = 0
+ var totalFailureCount = 0
val locations = getLocations(blockId)
- var numFetchFailures = 0
- for (loc <- locations) {
+ val maxFetchFailures = locations.size
+ var locationIterator = locations.iterator
+ while (locationIterator.hasNext) {
--- End diff --
just a thought: would this be easier to reason about as a tail-recursive method? would eliminate the need for the vars and returns.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186449606
**[Test build #51576 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51576/consoleFull)** for PR 11241 at commit [`6a5e7f5`](https://github.com/apache/spark/commit/6a5e7f57cd97d46d975f3176e24500afb54a71d1).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194510263
**[Test build #52759 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52759/consoleFull)** for PR 11241 at commit [`a43aa61`](https://github.com/apache/spark/commit/a43aa611bb8ccd3f331b832f2c6dc09b321c0a2d).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55605775
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -570,28 +577,49 @@ private[spark] class BlockManager(
preferredLocs ++ otherLocs
}
- private def doGetRemote(blockId: BlockId, asBlockResult: Boolean): Option[Any] = {
+ /**
+ * VisibleForTesting
+ */
+ private[spark] def doGetRemote(blockId: BlockId, blockTransferService: BlockTransferService,
+ asBlockResult: Boolean): Option[Any] = {
require(blockId != null, "BlockId is null")
+ var runningFailureCount = 0
+ var totalFailureCount = 0
val locations = getLocations(blockId)
- var numFetchFailures = 0
- for (loc <- locations) {
+ val maxFetchFailures = locations.size
+ var locationIterator = locations.iterator
+ while (locationIterator.hasNext) {
+ val loc = locationIterator.next()
logDebug(s"Getting remote block $blockId from $loc")
val data = try {
blockTransferService.fetchBlockSync(
loc.host, loc.port, loc.executorId, blockId.toString).nioByteBuffer()
} catch {
case NonFatal(e) =>
- numFetchFailures += 1
- if (numFetchFailures == locations.size) {
+ runningFailureCount += 1
+ totalFailureCount += 1
+
+ if (totalFailureCount >= maxFetchFailures) {
// An exception is thrown while fetching this block from all locations
--- End diff --
If I understand this correctly, I think this comments needs updating as well. You are no longer necessarily trying _all_ locations -- you're trying as many locations as you knew about initially, but (a) the number of locations might even be larger and (b) you might end up retrying the same locations after a refresh. Maybe something like:
Give up trying anymore locations. Either we've tried all of the original locations, or we've refreshed the list of locations from the master, and have still hit failures after trying locations from the refreshed list.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187397926
**[Test build #51675 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51675/consoleFull)** for PR 11241 at commit [`2412504`](https://github.com/apache/spark/commit/2412504634250fd2981922e15b22b2ead26f3ec0).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55604211
--- Diff: core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala ---
@@ -1331,6 +1336,72 @@ class BlockManagerSuite extends SparkFunSuite with Matchers with BeforeAndAfterE
assert(store.getSingle("a1").isDefined, "a1 was not in store")
assert(store.getSingle("a3").isDefined, "a3 was not in store")
}
+
+ test("SPARK-13328: refresh block locations (fetch should fail after hitting a threshold)") {
+ val mockBlockTransferService = new MockBlockTransferService(maxFailuresBeforeLocationRefresh)
+ store = makeBlockManager(8000, "executor1")
+
+ store.putSingle("item", 999L, StorageLevel.MEMORY_ONLY, tellMaster = true)
+
+ intercept[BlockFetchException] {
+ store.doGetRemote("item", mockBlockTransferService, asBlockResult = false)
+ .asInstanceOf[Option[ByteBuffer]]
+ }
+ }
+
+ test("SPARK-13328: refresh block locations (fetch should succeed after location refresh)") {
+ val mockBlockManagerMaster = mock(classOf[BlockManagerMaster])
+ val mockBlockTransferService = new MockBlockTransferService(maxFailuresBeforeLocationRefresh)
+
+ // make sure we have more than maxFailuresBeforeLocationRefresh locations
+ // so that we have a chance to do location refresh
+ val blockManagerIds = new ArrayBuffer[BlockManagerId]()
+ for (i <- 0 to maxFailuresBeforeLocationRefresh) {
+ blockManagerIds += BlockManagerId(s"id-$i", s"host-$i", i + 1)
+ }
--- End diff --
nit:
```
val blockManagerIds = (0 to maxFailuresBeforeLocationRefresh).map { i => BlockManagerId(s"id-$i", s"host-$i", i + 1) }
```
also I think you won't need `toSeq` on the next line then.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195145313
**[Test build #52867 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52867/consoleFull)** for PR 11241 at commit [`b418e13`](https://github.com/apache/spark/commit/b418e130d231ba4df386ef505443926719bd4b84).
* This patch passes all tests.
* This patch **does not merge 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195092208
Have you seen my latest comments about exposing fewer things for tests?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194605525
**[Test build #52774 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52774/consoleFull)** for PR 11241 at commit [`0383e6c`](https://github.com/apache/spark/commit/0383e6c3870cb38a31db334d3a2efcc39c6eb43a).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195230150
**[Test build #52901 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52901/consoleFull)** for PR 11241 at commit [`0875b24`](https://github.com/apache/spark/commit/0875b243ad2a17448bab742e8507818e49932a25).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55603252
--- Diff: core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala ---
@@ -1331,6 +1336,72 @@ class BlockManagerSuite extends SparkFunSuite with Matchers with BeforeAndAfterE
assert(store.getSingle("a1").isDefined, "a1 was not in store")
assert(store.getSingle("a3").isDefined, "a3 was not in store")
}
+
+ test("SPARK-13328: refresh block locations (fetch should fail after hitting a threshold)") {
+ val mockBlockTransferService = new MockBlockTransferService(maxFailuresBeforeLocationRefresh)
+ store = makeBlockManager(8000, "executor1")
+
+ store.putSingle("item", 999L, StorageLevel.MEMORY_ONLY, tellMaster = true)
+
+ intercept[BlockFetchException] {
+ store.doGetRemote("item", mockBlockTransferService, asBlockResult = false)
+ .asInstanceOf[Option[ByteBuffer]]
+ }
+ }
+
+ test("SPARK-13328: refresh block locations (fetch should succeed after location refresh)") {
+ val mockBlockManagerMaster = mock(classOf[BlockManagerMaster])
+ val mockBlockTransferService = new MockBlockTransferService(maxFailuresBeforeLocationRefresh)
+
+ // make sure we have more than maxFailuresBeforeLocationRefresh locations
+ // so that we have a chance to do location refresh
+ val blockManagerIds = new ArrayBuffer[BlockManagerId]()
+ for (i <- 0 to maxFailuresBeforeLocationRefresh) {
+ blockManagerIds += BlockManagerId(s"id-$i", s"host-$i", i + 1)
+ }
+
+ when(mockBlockManagerMaster.getLocations(mc.any[BlockId])).thenReturn(blockManagerIds.toSeq)
+
+ store = makeBlockManager(8000, "executor1", mockBlockManagerMaster)
+
+ val block = store.doGetRemote("item", mockBlockTransferService, asBlockResult = false)
+ .asInstanceOf[Option[ByteBuffer]]
+ assert(block.isDefined)
+ verify(mockBlockManagerMaster, times(2)).getLocations("item")
+ }
+
+ class MockBlockTransferService(val maxFailures: Int) extends BlockTransferService {
+ var numCalls = 0
+
+ override def init(blockDataManager: BlockDataManager): Unit = {}
+
+ override def fetchBlocks(host: String, port: Int, execId: String, blockIds: Array[String],
+ listener: BlockFetchingListener): Unit = {
+ listener.onBlockFetchSuccess("mockBlockId", new NioManagedBuffer(ByteBuffer.allocate(1)))
+ }
+
+ override def close(): Unit = {}
+
+ override def hostName: String = { "MockBlockTransferService::host" }
+
+ override def port: Int = { -1 }
+
+ override def uploadBlock(hostname: String, port: Int, execId: String,
+ blockId: BlockId, blockData: ManagedBuffer,
+ level: StorageLevel): Future[Unit] = {
+ import scala.concurrent.ExecutionContext.Implicits.global
+ Future {}
+ }
+
+ override def fetchBlockSync(host: String, port: Int,
+ execId: String, blockId: String): ManagedBuffer = {
+ numCalls += 1
+ if (numCalls <= maxFailures) {
+ throw new RuntimeException("Failing block fetch in the moch block transfer service")
--- End diff --
typo: mock
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r53528195
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -575,6 +582,18 @@ private[spark] class BlockManager(
// This location failed, so we retry fetch from a different one by returning null here
logWarning(s"Failed to fetch remote block $blockId " +
s"from $loc (failed attempt $numFetchFailures)", e)
+
+ // if dynamic alloc. is enabled and if there is a large number of executors
+ // then locations list can contain a large number of stale entries causing
+ // a large number of retries that may take a significant amount of time
+ // To get rid of these stale entries we refresh the block locations
+ // after a certain number of fetch failures
+ if (dynamicAllocationEnabled && numFetchFailures >= maxFailuresBeforeLocationRefresh) {
--- End diff --
I think we should do this whether or dynamic allocation is enabled
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r53203126
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -135,6 +135,10 @@ private[spark] class BlockManager(
// Whether to compress shuffle output temporarily spilled to disk
private val compressShuffleSpill = conf.getBoolean("spark.shuffle.spill.compress", true)
+ // Max number of failures before this block manager refreshes the block locations from the driver
+ private val maxFailuresBeforeLocationRefresh = conf.getInt("spark.block.failures.beforeLocationRefresh", Int.MaxValue)
--- End diff --
Set the default value to Int.MaxValue so that locations will not get refreshed by default, which I think is OK for small clusters. What do you think?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195182613
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187474717
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195105114
**[Test build #52869 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52869/consoleFull)** for PR 11241 at commit [`7ba025f`](https://github.com/apache/spark/commit/7ba025f8e16fb5335d05fb61dc36dd4026a6ae73).
* This patch **fails to build**.
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195102461
**[Test build #52868 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52868/consoleFull)** for PR 11241 at commit [`5bcf323`](https://github.com/apache/spark/commit/5bcf323f2ed2c927dfa9a646fe134561aae0a500).
* This patch **fails to build**.
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r53529282
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -135,6 +135,11 @@ private[spark] class BlockManager(
// Whether to compress shuffle output temporarily spilled to disk
private val compressShuffleSpill = conf.getBoolean("spark.shuffle.spill.compress", true)
+ // Max number of failures before this block manager refreshes the block locations from the driver
+ private val maxFailuresBeforeLocationRefresh =
+ conf.getInt("spark.block.failures.beforeLocationRefresh", Int.MaxValue)
--- End diff --
either that or a fraction of all the locations, like 0.5
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195230314
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/52901/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194561216
one more thought -- if you want to be sure to try all known locations before giving up, you could also try refreshing N times, and once you've decided not to refresh anymore, *then* go down the entire list before giving up for good. it does mean you're still stuck with a really long time to failure, but would help prevent the random failure mode you were worried about.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-185324180
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 project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186018396
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51510/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194560630
**[Test build #52774 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52774/consoleFull)** for PR 11241 at commit [`0383e6c`](https://github.com/apache/spark/commit/0383e6c3870cb38a31db334d3a2efcc39c6eb43a).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195075070
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/52851/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195023668
Looks great. My remaining comments are relatively minor. About making it configurable, it's probably OK as long as we don't also document it. I just don't want the user to have to think about their applications at this level of detail. We want Spark to be easy to use without a ton of tweaking. Maybe that's not really the case today but it's a goal we're striving towards.
(TL;DR keep the config but don't document 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 not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195075068
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186449812
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187473839
**[Test build #51700 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51700/consoleFull)** for PR 11241 at commit [`663e387`](https://github.com/apache/spark/commit/663e3877ddd0d34a0e2d8f3b296abe7cd04d4024).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187319511
@andrewor14 addressed your comments, can you please take a look?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186408535
**[Test build #51575 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51575/consoleFull)** for PR 11241 at commit [`f6fdfee`](https://github.com/apache/spark/commit/f6fdfeea5390f5c37981295c9d61fb29d3a258cb).
* This patch **fails Scala style 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194507703
**[Test build #52758 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52758/consoleFull)** for PR 11241 at commit [`b539f82`](https://github.com/apache/spark/commit/b539f82120cae22b7e7b3a6277e8394f17e2254c).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195096281
**[Test build #52867 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52867/consoleFull)** for PR 11241 at commit [`b418e13`](https://github.com/apache/spark/commit/b418e130d231ba4df386ef505443926719bd4b84).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194455011
@andrewor14 rebased. I agree with @tgravescs that this should be configurable (which was the case in my initial PR) but I am also fine as long as we get this in as this is an important issue for large deployment.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r53528127
--- Diff: core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala ---
@@ -1263,4 +1263,44 @@ class BlockManagerSuite extends SparkFunSuite with Matchers with BeforeAndAfterE
assert(result.size === 10000)
assert(result.data === Right(bytes))
}
+
+ test("SPARK-13328: refresh block locations from the driver if max fetch failures are reached") {
+ import scala.concurrent._
+ import ExecutionContext.Implicits.global
+
+ conf.set("spark.shuffle.io.maxRetries", "1")
+ conf.set("spark.shuffle.io.retryWait", "1s")
+ conf.set("spark.block.failures.beforeLocationRefresh", "1")
+ conf.set("spark.dynamicAllocation.enabled", "true")
+
+ store = makeBlockManager(8000, "executor1")
+ store2 = makeBlockManager(8000, "executor2")
+ store3 = makeBlockManager(8000, "executor3")
+
+ val item = 999L
+ store2.putSingle("item", item, StorageLevel.MEMORY_ONLY, tellMaster = true)
+ store3.putSingle("item", item, StorageLevel.MEMORY_ONLY, tellMaster = true)
+
+ assert(store.getRemoteBytes("item").isDefined, "block should be fetched properly")
+
+ store2.stop()
+ store2 = null
+
+ store3.stop()
+ store3 = null
+
+ val f = Future {
+ assert(store.getRemoteBytes("item").isDefined, "block should be fetched properly")
+ }
+
+ Thread.sleep(3000)
--- End diff --
this is a pretty brittle way to test this; the test may be flaky and it will take a long time to run it. Can you rewrite this in a way that's more of a unit test (e.g. by mocking)?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194965029
@tgravescs OK made it a config parameter (`spark.block.failures.beforeLocationRefresh` with a default of 5). After we hit this problem I have deployed this fix to production and the failing apps were able to complete successfully.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195102550
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/52868/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186408172
**[Test build #51575 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51575/consoleFull)** for PR 11241 at commit [`f6fdfee`](https://github.com/apache/spark/commit/f6fdfeea5390f5c37981295c9d61fb29d3a258cb).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55739701
--- Diff: core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala ---
@@ -1331,6 +1335,74 @@ class BlockManagerSuite extends SparkFunSuite with Matchers with BeforeAndAfterE
assert(store.getSingle("a1").isDefined, "a1 was not in store")
assert(store.getSingle("a3").isDefined, "a3 was not in store")
}
+
+ test("SPARK-13328: refresh block locations (fetch should fail after hitting a threshold)") {
+ store = makeBlockManager(8000, "executor1")
+ val mockBlockTransferService =
+ new MockBlockTransferService(store.maxFailuresBeforeLocationRefresh)
+
+ store.putSingle("item", 999L, StorageLevel.MEMORY_ONLY, tellMaster = true)
+
+ intercept[BlockFetchException] {
+ store.doGetRemote("item", mockBlockTransferService, asBlockResult = false)
+ .asInstanceOf[Option[ByteBuffer]]
+ }
+ }
+
+ test("SPARK-13328: refresh block locations (fetch should succeed after location refresh)") {
+ val mockBlockManagerMaster = mock(classOf[BlockManagerMaster])
+ store = makeBlockManager(8000, "executor1", mockBlockManagerMaster)
+ val mockBlockTransferService =
+ new MockBlockTransferService(store.maxFailuresBeforeLocationRefresh)
+
+ // make sure we have more than maxFailuresBeforeLocationRefresh locations
+ // so that we have a chance to do location refresh
+ val blockManagerIds = (0 to store.maxFailuresBeforeLocationRefresh)
+ .map { i => BlockManagerId(s"id-$i", s"host-$i", i + 1) }
+
+ when(mockBlockManagerMaster.getLocations(mc.any[BlockId])).thenReturn(blockManagerIds)
+
+ val block = store.doGetRemote("item", mockBlockTransferService, asBlockResult = false)
+ .asInstanceOf[Option[ByteBuffer]]
+ assert(block.isDefined)
+ verify(mockBlockManagerMaster, times(2)).getLocations("item")
+ }
+
+ class MockBlockTransferService(val maxFailures: Int) extends BlockTransferService {
+ var numCalls = 0
+
+ override def init(blockDataManager: BlockDataManager): Unit = {}
+
+ override def fetchBlocks(host: String, port: Int, execId: String, blockIds: Array[String],
+ listener: BlockFetchingListener): Unit = {
--- End diff --
style:
```
override def fetchBlocks(
host: String,
port: Int,
...,
listener: BlockFetchingListener): Unit
```
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194606136
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195105163
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195098262
@andrewor14 got rid of the whitespaces. Thanks everyone for their reviews.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187425086
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51690/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186449815
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51576/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195145524
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/52867/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55604303
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -570,28 +577,49 @@ private[spark] class BlockManager(
preferredLocs ++ otherLocs
}
- private def doGetRemote(blockId: BlockId, asBlockResult: Boolean): Option[Any] = {
+ /**
+ * VisibleForTesting
+ */
+ private[spark] def doGetRemote(blockId: BlockId, blockTransferService: BlockTransferService,
+ asBlockResult: Boolean): Option[Any] = {
--- End diff --
multiline style here too
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55740216
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -570,28 +576,53 @@ private[spark] class BlockManager(
preferredLocs ++ otherLocs
}
- private def doGetRemote(blockId: BlockId, asBlockResult: Boolean): Option[Any] = {
+ /**
+ * VisibleForTesting
+ */
+ private[spark] def doGetRemote(
+ blockId: BlockId,
+ blockTransferService: BlockTransferService,
--- End diff --
this interface is really clunky. I would refactor your tests a little to pass in your `MockBlockTransferService` when you `makeBlockManager`. Then (1) you don't need to expose this method and (2) you don't need to pass in a transfer service here, which is strange.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194508078
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/52758/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195096647
@andrewor14 just saw it and also rebased (seems like some changes have been pushed to master).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187401347
**[Test build #51686 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51686/consoleFull)** for PR 11241 at commit [`e444072`](https://github.com/apache/spark/commit/e4440727b32a7fd89ee8067a78814db60f6e486e).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187425078
**[Test build #51690 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51690/consoleFull)** for PR 11241 at commit [`44ec18b`](https://github.com/apache/spark/commit/44ec18b1724afbab8d18913dc5f1d6083f419a81).
* This patch **fails Scala style 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187424718
**[Test build #51690 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51690/consoleFull)** for PR 11241 at commit [`44ec18b`](https://github.com/apache/spark/commit/44ec18b1724afbab8d18913dc5f1d6083f419a81).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194439522
Looks like this needs to be upmerged. @nezihyigitbasi can you upmerge it to master.
I'm fine with leaving this hardcoded number for now if it gets this in. We can add a configuration later if needed.
@andrewor14 do you want to take another look at this?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195503487
Note to self: remember to close the issue once JIRA is back up
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195009737
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195154171
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194965160
**[Test build #52832 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52832/consoleFull)** for PR 11241 at commit [`e3045dd`](https://github.com/apache/spark/commit/e3045dd2fd041b312bd2c09eae433020002b1af6).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194559851
my comments are mostly minor style issues.
I'd prefer undocumented configurability as well, but also not super particular about it. from the sound of it, it also seems to me like the default ought to be really small, like maybe even 1 or 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55739719
--- Diff: core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala ---
@@ -1331,6 +1335,74 @@ class BlockManagerSuite extends SparkFunSuite with Matchers with BeforeAndAfterE
assert(store.getSingle("a1").isDefined, "a1 was not in store")
assert(store.getSingle("a3").isDefined, "a3 was not in store")
}
+
+ test("SPARK-13328: refresh block locations (fetch should fail after hitting a threshold)") {
+ store = makeBlockManager(8000, "executor1")
+ val mockBlockTransferService =
+ new MockBlockTransferService(store.maxFailuresBeforeLocationRefresh)
+
+ store.putSingle("item", 999L, StorageLevel.MEMORY_ONLY, tellMaster = true)
+
+ intercept[BlockFetchException] {
+ store.doGetRemote("item", mockBlockTransferService, asBlockResult = false)
+ .asInstanceOf[Option[ByteBuffer]]
+ }
+ }
+
+ test("SPARK-13328: refresh block locations (fetch should succeed after location refresh)") {
+ val mockBlockManagerMaster = mock(classOf[BlockManagerMaster])
+ store = makeBlockManager(8000, "executor1", mockBlockManagerMaster)
+ val mockBlockTransferService =
+ new MockBlockTransferService(store.maxFailuresBeforeLocationRefresh)
+
+ // make sure we have more than maxFailuresBeforeLocationRefresh locations
+ // so that we have a chance to do location refresh
+ val blockManagerIds = (0 to store.maxFailuresBeforeLocationRefresh)
+ .map { i => BlockManagerId(s"id-$i", s"host-$i", i + 1) }
+
+ when(mockBlockManagerMaster.getLocations(mc.any[BlockId])).thenReturn(blockManagerIds)
+
+ val block = store.doGetRemote("item", mockBlockTransferService, asBlockResult = false)
+ .asInstanceOf[Option[ByteBuffer]]
+ assert(block.isDefined)
+ verify(mockBlockManagerMaster, times(2)).getLocations("item")
+ }
+
+ class MockBlockTransferService(val maxFailures: Int) extends BlockTransferService {
+ var numCalls = 0
+
+ override def init(blockDataManager: BlockDataManager): Unit = {}
+
+ override def fetchBlocks(host: String, port: Int, execId: String, blockIds: Array[String],
+ listener: BlockFetchingListener): Unit = {
+ listener.onBlockFetchSuccess("mockBlockId", new NioManagedBuffer(ByteBuffer.allocate(1)))
+ }
+
+ override def close(): Unit = {}
+
+ override def hostName: String = { "MockBlockTransferService::host" }
+
+ override def port: Int = { -1 }
+
+ override def uploadBlock(
+ hostname: String,
+ port: Int, execId: String,
+ blockId: BlockId,
+ blockData: ManagedBuffer,
+ level: StorageLevel): Future[Unit] = {
+ import scala.concurrent.ExecutionContext.Implicits.global
+ Future {}
+ }
+
+ override def fetchBlockSync(host: String, port: Int,
+ execId: String, blockId: String): ManagedBuffer = {
--- End diff --
same here, style
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195503369
Merging into master, 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 this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187429853
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51697/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187440828
**[Test build #51700 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51700/consoleFull)** for PR 11241 at commit [`663e387`](https://github.com/apache/spark/commit/663e3877ddd0d34a0e2d8f3b296abe7cd04d4024).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187401877
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194510632
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/52759/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187412634
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186416073
**[Test build #51576 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51576/consoleFull)** for PR 11241 at commit [`6a5e7f5`](https://github.com/apache/spark/commit/6a5e7f57cd97d46d975f3176e24500afb54a71d1).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194456010
**[Test build #52759 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52759/consoleFull)** for PR 11241 at commit [`a43aa61`](https://github.com/apache/spark/commit/a43aa611bb8ccd3f331b832f2c6dc09b321c0a2d).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195145523
Build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/11241
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r53531237
--- Diff: core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala ---
@@ -1263,4 +1263,44 @@ class BlockManagerSuite extends SparkFunSuite with Matchers with BeforeAndAfterE
assert(result.size === 10000)
assert(result.data === Right(bytes))
}
+
+ test("SPARK-13328: refresh block locations from the driver if max fetch failures are reached") {
+ import scala.concurrent._
+ import ExecutionContext.Implicits.global
+
+ conf.set("spark.shuffle.io.maxRetries", "1")
+ conf.set("spark.shuffle.io.retryWait", "1s")
+ conf.set("spark.block.failures.beforeLocationRefresh", "1")
+ conf.set("spark.dynamicAllocation.enabled", "true")
+
+ store = makeBlockManager(8000, "executor1")
+ store2 = makeBlockManager(8000, "executor2")
+ store3 = makeBlockManager(8000, "executor3")
+
+ val item = 999L
+ store2.putSingle("item", item, StorageLevel.MEMORY_ONLY, tellMaster = true)
+ store3.putSingle("item", item, StorageLevel.MEMORY_ONLY, tellMaster = true)
+
+ assert(store.getRemoteBytes("item").isDefined, "block should be fetched properly")
+
+ store2.stop()
+ store2 = null
+
+ store3.stop()
+ store3 = null
+
+ val f = Future {
+ assert(store.getRemoteBytes("item").isDefined, "block should be fetched properly")
+ }
+
+ Thread.sleep(3000)
--- End diff --
I don't also like depending on timing, but couldn't really find a decent way to trigger this code path (a case where a previously failing block fetch succeeds after a refresh). Which component do you propose to mock?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186438421
@nezihyigitbasi thanks for explaining the issue concisely in the description. I can see how this patch fixes it, but as I mentioned in my comments I think we should just make the refresh threshold a constant instead of allowing the user to set it. Another concern I have is that whatever solution we come up with here we need to make sure we never go into an infinite loop. It's hard to prove that this patch in its current state does not potentially introduce one.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194453762
**[Test build #52758 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52758/consoleFull)** for PR 11241 at commit [`b539f82`](https://github.com/apache/spark/commit/b539f82120cae22b7e7b3a6277e8394f17e2254c).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187429521
**[Test build #51697 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51697/consoleFull)** for PR 11241 at commit [`b67bf56`](https://github.com/apache/spark/commit/b67bf56f6037de3fde8eb51f53f71ae2b02d8587).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195098989
**[Test build #52869 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52869/consoleFull)** for PR 11241 at commit [`7ba025f`](https://github.com/apache/spark/commit/7ba025f8e16fb5335d05fb61dc36dd4026a6ae73).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r53529534
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -135,6 +135,11 @@ private[spark] class BlockManager(
// Whether to compress shuffle output temporarily spilled to disk
private val compressShuffleSpill = conf.getBoolean("spark.shuffle.spill.compress", true)
+ // Max number of failures before this block manager refreshes the block locations from the driver
+ private val maxFailuresBeforeLocationRefresh =
+ conf.getInt("spark.block.failures.beforeLocationRefresh", Int.MaxValue)
--- End diff --
If we really want to make this constant I think it should be < 10, because with the default retry wait/count settings 10 failures correspond to 15*10 = 2.5 minutes just looping through removed executors.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187793223
@andrewor14 addressed your comments && tests have passed, can you please take a look?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195183204
**[Test build #52901 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52901/consoleFull)** for PR 11241 at commit [`0875b24`](https://github.com/apache/spark/commit/0875b243ad2a17448bab742e8507818e49932a25).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195097244
**[Test build #52868 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52868/consoleFull)** for PR 11241 at commit [`5bcf323`](https://github.com/apache/spark/commit/5bcf323f2ed2c927dfa9a646fe134561aae0a500).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195102546
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55740272
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -137,6 +137,10 @@ private[spark] class BlockManager(
private val compressRdds = conf.getBoolean("spark.rdd.compress", false)
// Whether to compress shuffle output temporarily spilled to disk
private val compressShuffleSpill = conf.getBoolean("spark.shuffle.spill.compress", true)
+ // Max number of failures before this block manager refreshes the block locations from the driver
+ // VisibleForTesting
+ private[spark] val maxFailuresBeforeLocationRefresh =
--- End diff --
You can also make this fully private if you just get it from the conf in tests
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186400398
Updated to fix the style problems.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187429842
**[Test build #51697 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51697/consoleFull)** for PR 11241 at commit [`b67bf56`](https://github.com/apache/spark/commit/b67bf56f6037de3fde8eb51f53f71ae2b02d8587).
* This patch **fails Scala style 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186408541
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r53528644
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -135,6 +135,11 @@ private[spark] class BlockManager(
// Whether to compress shuffle output temporarily spilled to disk
private val compressShuffleSpill = conf.getBoolean("spark.shuffle.spill.compress", true)
+ // Max number of failures before this block manager refreshes the block locations from the driver
+ private val maxFailuresBeforeLocationRefresh =
+ conf.getInt("spark.block.failures.beforeLocationRefresh", Int.MaxValue)
--- End diff --
I would just make this a constant instead of introducing another config:
```
private[spark] object BlockManager {
/**
* Max number of failures when fetching a block remotely before refreshing the block's
* locations from the driver. This is useful for cases where many executors fail at once,
* e.g. dynamic allocation. (SPARK-13328)
*/
private val REFRESH_LOCATION_FETCH_FAILURE_THRESHOLD = 10
}
```
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195009339
**[Test build #52832 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52832/consoleFull)** for PR 11241 at commit [`e3045dd`](https://github.com/apache/spark/commit/e3045dd2fd041b312bd2c09eae433020002b1af6).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187429848
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194508076
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194658109
So I think we should make this configurable. We can leave it as undocumented config but at least with it as a config users can have a fall back and tune as their job needs or set really high to get previous behavior.
@nezihyigitbasi have you done any manually testing with real applications on this?
I'm not thinking of any cases where trying all locations would necessarily help (it would be possible but not sure how likely) but I'll think about it some more tomorrow.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r53532737
--- Diff: core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala ---
@@ -1263,4 +1263,44 @@ class BlockManagerSuite extends SparkFunSuite with Matchers with BeforeAndAfterE
assert(result.size === 10000)
assert(result.data === Right(bytes))
}
+
+ test("SPARK-13328: refresh block locations from the driver if max fetch failures are reached") {
+ import scala.concurrent._
+ import ExecutionContext.Implicits.global
+
+ conf.set("spark.shuffle.io.maxRetries", "1")
+ conf.set("spark.shuffle.io.retryWait", "1s")
+ conf.set("spark.block.failures.beforeLocationRefresh", "1")
+ conf.set("spark.dynamicAllocation.enabled", "true")
+
+ store = makeBlockManager(8000, "executor1")
+ store2 = makeBlockManager(8000, "executor2")
+ store3 = makeBlockManager(8000, "executor3")
+
+ val item = 999L
+ store2.putSingle("item", item, StorageLevel.MEMORY_ONLY, tellMaster = true)
+ store3.putSingle("item", item, StorageLevel.MEMORY_ONLY, tellMaster = true)
+
+ assert(store.getRemoteBytes("item").isDefined, "block should be fetched properly")
+
+ store2.stop()
+ store2 = null
+
+ store3.stop()
+ store3 = null
+
+ val f = Future {
+ assert(store.getRemoteBytes("item").isDefined, "block should be fetched properly")
+ }
+
+ Thread.sleep(3000)
--- End diff --
well one thing you could do is pass in your own custom `BlockTransferService` that overrides `fetchBlockSync` to throw exceptions if it's the first N block managers. Then you can use Mockito `verify` to check how many times `BlockManager#getLocations` was called.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195024189
**[Test build #52851 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52851/consoleFull)** for PR 11241 at commit [`bba6d4c`](https://github.com/apache/spark/commit/bba6d4cf15ae9539b7a37876deb197a93ab4b3d0).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55604381
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -58,6 +58,11 @@ private case object DoPutSucceeded extends DoPutResult
private case object DoPutBytesFailed extends DoPutResult
private case class DoPutIteratorFailed(iter: Iterator[Any]) extends DoPutResult
+private[spark] object BlockManagerConfiguration {
+ // Max number of failures before this block manager refreshes the block locations from the driver
+ def maxFailuresBeforeLocationRefresh: Int = 5
+}
--- End diff --
I don't think we really need a separate config object for this, you could put it in the existing `object BlockManager`
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55767768
--- Diff: core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala ---
@@ -1287,6 +1291,84 @@ class BlockManagerSuite extends SparkFunSuite with Matchers with BeforeAndAfterE
assert(store.getSingle("a1").isDefined, "a1 was not in store")
assert(store.getSingle("a3").isDefined, "a3 was not in store")
}
+
+ test("SPARK-13328: refresh block locations (fetch should fail after hitting a threshold)") {
+ store = makeBlockManager(8000, "executor1")
+ val mockBlockTransferService =
+ new MockBlockTransferService(conf.getInt("spark.block.failures.beforeLocationRefresh", 5))
+
+ store.putSingle("item", 999L, StorageLevel.MEMORY_ONLY, tellMaster = true)
+
+ intercept[BlockFetchException] {
+ store.doGetRemote("item", mockBlockTransferService, asBlockResult = false)
+ }
+ }
+
+ test("SPARK-13328: refresh block locations (fetch should succeed after location refresh)") {
+ val maxFailuresBeforeLocationRefresh =
+ conf.getInt("spark.block.failures.beforeLocationRefresh", 5)
+
+ val mockBlockManagerMaster = mock(classOf[BlockManagerMaster])
+ val mockBlockTransferService =
+ new MockBlockTransferService(maxFailuresBeforeLocationRefresh)
+
+ // make sure we have more than maxFailuresBeforeLocationRefresh locations
+ // so that we have a chance to do location refresh
+ val blockManagerIds = (0 to maxFailuresBeforeLocationRefresh)
+ .map { i => BlockManagerId(s"id-$i", s"host-$i", i + 1) }
+
+ when(mockBlockManagerMaster.getLocations(mc.any[BlockId])).thenReturn(blockManagerIds)
+
--- End diff --
style nit: can you remove all these empty blank lines within a test?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187425082
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187398701
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51675/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186017906
**[Test build #51510 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51510/consoleFull)** for PR 11241 at commit [`45bdec6`](https://github.com/apache/spark/commit/45bdec651a2d15ad97638a676675c1697ddade09).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187474723
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51700/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r53532327
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -575,6 +582,18 @@ private[spark] class BlockManager(
// This location failed, so we retry fetch from a different one by returning null here
logWarning(s"Failed to fetch remote block $blockId " +
s"from $loc (failed attempt $numFetchFailures)", e)
+
+ // if dynamic alloc. is enabled and if there is a large number of executors
+ // then locations list can contain a large number of stale entries causing
+ // a large number of retries that may take a significant amount of time
+ // To get rid of these stale entries we refresh the block locations
+ // after a certain number of fetch failures
+ if (dynamicAllocationEnabled && numFetchFailures >= maxFailuresBeforeLocationRefresh) {
--- End diff --
we can set the threshold equal to the initial number of locations, i.e.
```
val initialLocations = getLocations(blockId)
val maxFetchFailures = initialLocations.size
val locationIter = initialLocations.iterator
while(locationIter.hasNext) { ... }
```
Essentially this says that we never traverse more executors than we started with. It's like saying we never do more work than we would have if we had not refreshed the locations.
Regardless of the actual threshold we choose I do feel strongly that we need to break out of this loop at some point. Infinite loops are one of the hardest things to debug.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194560497
@squito thanks for the review, addressed your comments. I think the only thing that is left to merge this issue is to reach consensus on whether we want to make it configurable, and if no what the default value should be.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55739189
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -570,28 +576,53 @@ private[spark] class BlockManager(
preferredLocs ++ otherLocs
}
- private def doGetRemote(blockId: BlockId, asBlockResult: Boolean): Option[Any] = {
+ /**
+ * VisibleForTesting
+ */
+ private[spark] def doGetRemote(
+ blockId: BlockId,
+ blockTransferService: BlockTransferService,
+ asBlockResult: Boolean): Option[Any] = {
require(blockId != null, "BlockId is null")
+ var runningFailureCount = 0
+ var totalFailureCount = 0
val locations = getLocations(blockId)
- var numFetchFailures = 0
- for (loc <- locations) {
+ val maxFetchFailures = locations.size
+ var locationIterator = locations.iterator
+ while (locationIterator.hasNext) {
+ val loc = locationIterator.next()
logDebug(s"Getting remote block $blockId from $loc")
val data = try {
blockTransferService.fetchBlockSync(
loc.host, loc.port, loc.executorId, blockId.toString).nioByteBuffer()
} catch {
case NonFatal(e) =>
- numFetchFailures += 1
- if (numFetchFailures == locations.size) {
- // An exception is thrown while fetching this block from all locations
- throw new BlockFetchException(s"Failed to fetch block from" +
- s" ${locations.size} locations. Most recent failure cause:", e)
- } else {
- // This location failed, so we retry fetch from a different one by returning null here
- logWarning(s"Failed to fetch remote block $blockId " +
- s"from $loc (failed attempt $numFetchFailures)", e)
- null
+ runningFailureCount += 1
+ totalFailureCount += 1
+
+ if (totalFailureCount >= maxFetchFailures) {
+ // Give up trying anymore locations. Either we've tried all of the original locations,
+ // or we've refreshed the list of locations from the master, and have still
+ // hit failures after trying locations from the refreshed list.
+ throw new BlockFetchException(s"Failed to fetch block after" +
+ s" ${totalFailureCount} fetch failures. Most recent failure cause:", e)
+ }
+
+ // This location failed, so we retry fetch from a different one by returning null here
--- End diff --
this comment should appear above `null` down there
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195230312
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186444001
@andrewor14 thanks for taking a look. We can introduce a global failure threshold to break out, but do we really want that global threshold to be a constant? Because it's possible that from run to run with the same settings one run can succeed and the other one can fail (hit the threshold) depending on the order of the live/removed executors in the location list (tl;dr from a user's point of view a job can arbitrarily fail from run to run).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55767709
--- Diff: core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala ---
@@ -1287,6 +1291,84 @@ class BlockManagerSuite extends SparkFunSuite with Matchers with BeforeAndAfterE
assert(store.getSingle("a1").isDefined, "a1 was not in store")
assert(store.getSingle("a3").isDefined, "a3 was not in store")
}
+
+ test("SPARK-13328: refresh block locations (fetch should fail after hitting a threshold)") {
+ store = makeBlockManager(8000, "executor1")
+ val mockBlockTransferService =
+ new MockBlockTransferService(conf.getInt("spark.block.failures.beforeLocationRefresh", 5))
+
+ store.putSingle("item", 999L, StorageLevel.MEMORY_ONLY, tellMaster = true)
+
+ intercept[BlockFetchException] {
+ store.doGetRemote("item", mockBlockTransferService, asBlockResult = false)
+ }
+ }
+
+ test("SPARK-13328: refresh block locations (fetch should succeed after location refresh)") {
+ val maxFailuresBeforeLocationRefresh =
+ conf.getInt("spark.block.failures.beforeLocationRefresh", 5)
+
+ val mockBlockManagerMaster = mock(classOf[BlockManagerMaster])
+ val mockBlockTransferService =
+ new MockBlockTransferService(maxFailuresBeforeLocationRefresh)
+
+ // make sure we have more than maxFailuresBeforeLocationRefresh locations
+ // so that we have a chance to do location refresh
+ val blockManagerIds = (0 to maxFailuresBeforeLocationRefresh)
+ .map { i => BlockManagerId(s"id-$i", s"host-$i", i + 1) }
+
+ when(mockBlockManagerMaster.getLocations(mc.any[BlockId])).thenReturn(blockManagerIds)
+
+ store = makeBlockManager(8000, "executor1", mockBlockManagerMaster)
+
+ val block = store.doGetRemote("item", mockBlockTransferService, asBlockResult = false)
+ .asInstanceOf[Option[ByteBuffer]]
+ assert(block.isDefined)
+ verify(mockBlockManagerMaster, times(2)).getLocations("item")
+ }
+
+ class MockBlockTransferService(val maxFailures: Int) extends BlockTransferService {
+ var numCalls = 0
+
+ override def init(blockDataManager: BlockDataManager): Unit = {}
+
+ override def fetchBlocks(
+ host: String,
+ port: Int,
+ execId: String,
+ blockIds: Array[String],
+ listener: BlockFetchingListener): Unit = {
+ listener.onBlockFetchSuccess("mockBlockId", new NioManagedBuffer(ByteBuffer.allocate(1)))
+ }
+
+ override def close(): Unit = {}
+
+ override def hostName: String = { "MockBlockTransferService::host" }
+
+ override def port: Int = { -1 }
+
+ override def uploadBlock(
+ hostname: String,
+ port: Int, execId: String,
+ blockId: BlockId,
+ blockData: ManagedBuffer,
+ level: StorageLevel): Future[Unit] = {
+ import scala.concurrent.ExecutionContext.Implicits.global
+ Future {}
+ }
+
+ override def fetchBlockSync(
--- End diff --
you have an extra space 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-191389153
@andrewor14 do you have any other comments for this PR?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r53529137
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -575,6 +582,18 @@ private[spark] class BlockManager(
// This location failed, so we retry fetch from a different one by returning null here
logWarning(s"Failed to fetch remote block $blockId " +
s"from $loc (failed attempt $numFetchFailures)", e)
+
+ // if dynamic alloc. is enabled and if there is a large number of executors
+ // then locations list can contain a large number of stale entries causing
+ // a large number of retries that may take a significant amount of time
+ // To get rid of these stale entries we refresh the block locations
+ // after a certain number of fetch failures
+ if (dynamicAllocationEnabled && numFetchFailures >= maxFailuresBeforeLocationRefresh) {
--- End diff --
also, is there danger that this will never terminate? What if `maxFailuresBeforeLocationRefresh` was set to 10 when number of executors is like 1000, then we'll literally never throw the `BlockFetchException`. Maybe we need to keep track of a global `numFetchFailures` to throw that exception based on some threshold.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187398688
**[Test build #51675 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51675/consoleFull)** for PR 11241 at commit [`2412504`](https://github.com/apache/spark/commit/2412504634250fd2981922e15b22b2ead26f3ec0).
* This patch **fails Scala style 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194510631
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186011256
ok to test
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186408545
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51575/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55564719
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -555,28 +563,49 @@ private[spark] class BlockManager(
preferredLocs ++ otherLocs
}
- private def doGetRemote(blockId: BlockId, asBlockResult: Boolean): Option[Any] = {
+ /**
+ * VisibleForTesting
+ */
+ private[spark] def doGetRemote(blockId: BlockId, blockTransferService: BlockTransferService,
+ asBlockResult: Boolean): Option[Any] = {
require(blockId != null, "BlockId is null")
+ var runningFailureCount = 0
+ var totalFailureCount = 0
val locations = getLocations(blockId)
- var numFetchFailures = 0
- for (loc <- locations) {
+ val maxFetchFailures = locations.size
+ var locationIterator = locations.iterator
+ while (locationIterator.hasNext) {
+ val loc = locationIterator.next()
logDebug(s"Getting remote block $blockId from $loc")
val data = try {
blockTransferService.fetchBlockSync(
loc.host, loc.port, loc.executorId, blockId.toString).nioByteBuffer()
} catch {
case NonFatal(e) =>
- numFetchFailures += 1
- if (numFetchFailures == locations.size) {
+ runningFailureCount += 1
+ totalFailureCount += 1
+
+ if (totalFailureCount >= maxFetchFailures) {
// An exception is thrown while fetching this block from all locations
- throw new BlockFetchException(s"Failed to fetch block from" +
- s" ${locations.size} locations. Most recent failure cause:", e)
- } else {
- // This location failed, so we retry fetch from a different one by returning null here
- logWarning(s"Failed to fetch remote block $blockId " +
- s"from $loc (failed attempt $numFetchFailures)", e)
- null
+ throw new BlockFetchException(s"Failed to fetch block after" +
+ s" ${totalFailureCount} fetch failures. Most recent failure cause:", e)
+ }
+
+ // This location failed, so we retry fetch from a different one by returning null here
+ logWarning(s"Failed to fetch remote block $blockId " +
+ s"from $loc (failed attempt $runningFailureCount)", e)
+
+ // If there is a large number of executors then locations list can contain a
+ // large number of stale entries causing a large number of retries that may
+ // take a significant amount of time. To get rid of these stale entries
+ // we refresh the block locations after a certain number of fetch failures
+ if (runningFailureCount >= BlockManagerConfiguration.maxFailuresBeforeLocationRefresh) {
+ locationIterator = Random.shuffle(master.getLocations(blockId)).iterator
--- End diff --
there is a new function getLocations() that should be used.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195009738
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/52832/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195096863
LGTM once you rebase I'll go ahead and merge 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 not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55739815
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -570,28 +576,53 @@ private[spark] class BlockManager(
preferredLocs ++ otherLocs
}
- private def doGetRemote(blockId: BlockId, asBlockResult: Boolean): Option[Any] = {
+ /**
+ * VisibleForTesting
+ */
--- End diff --
nit: just do `// VisibleForTesting`. This isn't really a javadoc
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55739607
--- Diff: core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala ---
@@ -1331,6 +1335,74 @@ class BlockManagerSuite extends SparkFunSuite with Matchers with BeforeAndAfterE
assert(store.getSingle("a1").isDefined, "a1 was not in store")
assert(store.getSingle("a3").isDefined, "a3 was not in store")
}
+
+ test("SPARK-13328: refresh block locations (fetch should fail after hitting a threshold)") {
+ store = makeBlockManager(8000, "executor1")
+ val mockBlockTransferService =
+ new MockBlockTransferService(store.maxFailuresBeforeLocationRefresh)
+
+ store.putSingle("item", 999L, StorageLevel.MEMORY_ONLY, tellMaster = true)
+
+ intercept[BlockFetchException] {
+ store.doGetRemote("item", mockBlockTransferService, asBlockResult = false)
+ .asInstanceOf[Option[ByteBuffer]]
--- End diff --
we don't need this cast right?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-194606140
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/52774/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195153889
**[Test build #52871 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52871/consoleFull)** for PR 11241 at commit [`0875b24`](https://github.com/apache/spark/commit/0875b243ad2a17448bab742e8507818e49932a25).
* This patch **fails PySpark 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195023960
@andrewor14 comments addressed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-187401879
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51686/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186018394
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-195074633
**[Test build #52851 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/52851/consoleFull)** for PR 11241 at commit [`bba6d4c`](https://github.com/apache/spark/commit/bba6d4cf15ae9539b7a37876deb197a93ab4b3d0).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r55603906
--- Diff: core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala ---
@@ -1331,6 +1336,72 @@ class BlockManagerSuite extends SparkFunSuite with Matchers with BeforeAndAfterE
assert(store.getSingle("a1").isDefined, "a1 was not in store")
assert(store.getSingle("a3").isDefined, "a3 was not in store")
}
+
+ test("SPARK-13328: refresh block locations (fetch should fail after hitting a threshold)") {
+ val mockBlockTransferService = new MockBlockTransferService(maxFailuresBeforeLocationRefresh)
+ store = makeBlockManager(8000, "executor1")
+
+ store.putSingle("item", 999L, StorageLevel.MEMORY_ONLY, tellMaster = true)
+
+ intercept[BlockFetchException] {
+ store.doGetRemote("item", mockBlockTransferService, asBlockResult = false)
+ .asInstanceOf[Option[ByteBuffer]]
+ }
+ }
+
+ test("SPARK-13328: refresh block locations (fetch should succeed after location refresh)") {
+ val mockBlockManagerMaster = mock(classOf[BlockManagerMaster])
+ val mockBlockTransferService = new MockBlockTransferService(maxFailuresBeforeLocationRefresh)
+
+ // make sure we have more than maxFailuresBeforeLocationRefresh locations
+ // so that we have a chance to do location refresh
+ val blockManagerIds = new ArrayBuffer[BlockManagerId]()
+ for (i <- 0 to maxFailuresBeforeLocationRefresh) {
+ blockManagerIds += BlockManagerId(s"id-$i", s"host-$i", i + 1)
+ }
+
+ when(mockBlockManagerMaster.getLocations(mc.any[BlockId])).thenReturn(blockManagerIds.toSeq)
+
+ store = makeBlockManager(8000, "executor1", mockBlockManagerMaster)
+
+ val block = store.doGetRemote("item", mockBlockTransferService, asBlockResult = false)
+ .asInstanceOf[Option[ByteBuffer]]
+ assert(block.isDefined)
+ verify(mockBlockManagerMaster, times(2)).getLocations("item")
+ }
+
+ class MockBlockTransferService(val maxFailures: Int) extends BlockTransferService {
+ var numCalls = 0
+
+ override def init(blockDataManager: BlockDataManager): Unit = {}
+
+ override def fetchBlocks(host: String, port: Int, execId: String, blockIds: Array[String],
+ listener: BlockFetchingListener): Unit = {
+ listener.onBlockFetchSuccess("mockBlockId", new NioManagedBuffer(ByteBuffer.allocate(1)))
+ }
+
+ override def close(): Unit = {}
+
+ override def hostName: String = { "MockBlockTransferService::host" }
+
+ override def port: Int = { -1 }
+
+ override def uploadBlock(hostname: String, port: Int, execId: String,
+ blockId: BlockId, blockData: ManagedBuffer,
+ level: StorageLevel): Future[Unit] = {
--- End diff --
on all of these methods, the style for multiline arg defs is each arg on its own line, indented 4 spaces, like:
```
override def uploadBlock(
hostname: String,
port: Int,
execId: String,
blockId: BlockId,
blockData: ManagedBuffer,
level: StorageLevel): Future[Unit] = {
```
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by nezihyigitbasi <gi...@git.apache.org>.
Github user nezihyigitbasi commented on a diff in the pull request:
https://github.com/apache/spark/pull/11241#discussion_r53530728
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala ---
@@ -575,6 +582,18 @@ private[spark] class BlockManager(
// This location failed, so we retry fetch from a different one by returning null here
logWarning(s"Failed to fetch remote block $blockId " +
s"from $loc (failed attempt $numFetchFailures)", e)
+
+ // if dynamic alloc. is enabled and if there is a large number of executors
+ // then locations list can contain a large number of stale entries causing
+ // a large number of retries that may take a significant amount of time
+ // To get rid of these stale entries we refresh the block locations
+ // after a certain number of fetch failures
+ if (dynamicAllocationEnabled && numFetchFailures >= maxFailuresBeforeLocationRefresh) {
--- End diff --
Eventually it should terminate as this loop should eventually hit a live replica as the locations are refreshed constantly. I thought about setting a global threshold, but it is tricky. We can keep refreshing locations and keep track of the fetch failures, but it's possible that we can hit the threshold before hitting a live replica (even if it exists in the location list -- that's one of the reasons this logic just keeps going on to eventually hit a live replica). Because I have seen that if there is a lot of executors coming up and going away we still get multiple stale locations with every location refresh and depending on the threshold value we may not even hit a live executor in this list.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13328][Core]: Poor read performance for...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11241#issuecomment-186018383
**[Test build #51510 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51510/consoleFull)** for PR 11241 at commit [`45bdec6`](https://github.com/apache/spark/commit/45bdec651a2d15ad97638a676675c1697ddade09).
* This patch **fails Scala style 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org