[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-07 Thread cenyuhai
Github user cenyuhai closed the pull request at:

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


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread andrewor14
Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217566942
  
@cenyuhai this is now merged. Can you close 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread andrewor14
Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217555196
  
Thanks, I'm merging this into branch-1.6.


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

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


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217549030
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread SparkQA
Github user SparkQA commented on the pull request:

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


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread andrewor14
Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217523810
  
There was an incorrect merge conflict 
https://github.com/apache/spark/commit/a3aa22a5915c2cc6bdd6810227a3698c59823009 
in the scheduler so maybe that is related.


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217523541
  
**[Test build #58014 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58014/consoleFull)**
 for PR 11546 at commit 
[`27fd070`](https://github.com/apache/spark/commit/27fd07058112dad0760c7fa5480fb43d4f046d96).


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread andrewor14
Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217523360
  
The changes themselves LGTM. I'm not sure why it timed out so let's try 
again.


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread andrewor14
Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217522610
  
retest this please


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread cenyuhai
Github user cenyuhai commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217467789
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread cenyuhai
Github user cenyuhai commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217466944
  
@andrewor14 I alter the code as what you said, but the test failed because 
of timeout. It seems like that it is none of my business...


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217456515
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

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


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217456396
  
**[Test build #57982 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57982/consoleFull)**
 for PR 11546 at commit 
[`27fd070`](https://github.com/apache/spark/commit/27fd07058112dad0760c7fa5480fb43d4f046d96).
 * This patch **fails from timeout after a configured wait of \`250m\`**.
 * This patch merges cleanly.
 * This patch adds no public classes.


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217405381
  
**[Test build #57982 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57982/consoleFull)**
 for PR 11546 at commit 
[`27fd070`](https://github.com/apache/spark/commit/27fd07058112dad0760c7fa5480fb43d4f046d96).


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217395768
  
**[Test build #57975 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57975/consoleFull)**
 for PR 11546 at commit 
[`a096afa`](https://github.com/apache/spark/commit/a096afa9e1f6105fe55d481130d24f1d0ad9cb26).
 * This patch **fails R style 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

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


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217395770
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-06 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217395116
  
**[Test build #57975 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57975/consoleFull)**
 for PR 11546 at commit 
[`a096afa`](https://github.com/apache/spark/commit/a096afa9e1f6105fe55d481130d24f1d0ad9cb26).


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-05 Thread andrewor14
Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217323897
  
@cenyuhai Thanks for fixing this issue. The two offending locks here seem 
to be `info` and `memoryManager`, which wrap each other in one order in 
`removeBlock` and the opposite order in `evictBlocksToFreeSpace`. I believe 
your fix here is largely correct but still not completely safe.

Once you address the comments I left I will go ahead and merge 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-05 Thread andrewor14
Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/11546#discussion_r62279240
  
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala 
---
@@ -1108,11 +1118,14 @@ private[spark] class BlockManager(
   def removeBlock(blockId: BlockId, tellMaster: Boolean = true): Unit = {
 logDebug(s"Removing block $blockId")
 val info = blockInfo.get(blockId).orNull
-if (info != null) {
+if (info != null && !pendingToRemove.containsKey(blockId)) {
+  pendingToRemove.put(blockId, currentTaskAttemptId)
--- End diff --

something like
```
if (info != null && pendingToRemove.putIfAbsent(blockId, 
currentTaskAttemptId) == null) {
```


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-05 Thread andrewor14
Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/11546#discussion_r62279170
  
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala 
---
@@ -1108,11 +1118,14 @@ private[spark] class BlockManager(
   def removeBlock(blockId: BlockId, tellMaster: Boolean = true): Unit = {
 logDebug(s"Removing block $blockId")
 val info = blockInfo.get(blockId).orNull
-if (info != null) {
+if (info != null && !pendingToRemove.containsKey(blockId)) {
+  pendingToRemove.put(blockId, currentTaskAttemptId)
--- End diff --

Actually, on closer inspection I believe this is still not safe. You need 
to use something like `pendingToRemove.putIfAbsent` to do this atomically.


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-05 Thread andrewor14
Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217323209
  
@cenyuhai Thanks for fixing this issue. If I understand correctly the 
deadlock happens when we unpersist an RDD or a broadcast (manually or through 
the cleaner) while we evict blocks from memory to free space for another block. 
The two offending locks here seem to be `info` and `memoryManager`, which wrap 
each other in one order in `removeBlock` and the opposite order in 
`evictBlocksToFreeSpace`. Your fix here seems correct and largely surgical. 
Once you address the comments I left I will go ahead and merge 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-05 Thread andrewor14
Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/11546#discussion_r62278680
  
--- Diff: 
core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala ---
@@ -424,6 +424,32 @@ class BlockManagerSuite extends SparkFunSuite with 
Matchers with BeforeAndAfterE
 }
   }
 
+  test("deadlock between dropFromMemory and removeBlock") {
+store = makeBlockManager(2000)
+val a1 = new Array[Byte](400)
+store.putSingle("a1", a1, StorageLevel.MEMORY_ONLY)
+val t1 = new Thread {
+  override def run() = {
+store.memoryManager.synchronized {
+  Thread.sleep(1000)
--- End diff --

I was able to reproduce the deadlock locally with this test almost every 
time, which is great. However, it would be good if you could rewrite this using 
explicit locks or semaphores or something so we don't have to introduce 
`Thread.sleep` here, which slows the test down and is more brittle.


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-05 Thread andrewor14
Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/11546#discussion_r62278386
  
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala 
---
@@ -1108,11 +1118,14 @@ private[spark] class BlockManager(
   def removeBlock(blockId: BlockId, tellMaster: Boolean = true): Unit = {
 logDebug(s"Removing block $blockId")
 val info = blockInfo.get(blockId).orNull
-if (info != null) {
+if (info != null && !pendingToRemove.containsKey(blockId)) {
+  pendingToRemove.put(blockId, currentTaskAttemptId)
   info.synchronized {
+val level = info.level
 // Removals are idempotent in disk store and memory store. At 
worst, we get a warning.
-val removedFromMemory = memoryStore.remove(blockId)
-val removedFromDisk = diskStore.remove(blockId)
+val removedFromMemory = if (level.useMemory) 
memoryStore.remove(blockId) else false
+pendingToRemove.remove(blockId)
--- End diff --

we should wrap this in a try finally in case an exception is thrown. 
Otherwise we may never release the "pending" lock:
```
if (info != null && !pendingToRemove.containsKey(blockId)) {
  pendingToRemove.put(blockId, currentTaskAttemptId)
  try {
info.synchronized {
  ... // remove the block from memoryStore
}
  } finally {
pendingToRemove.remove(blockId)
  }
}
```
Here and a few other places.


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-05 Thread andrewor14
Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/11546#discussion_r62278278
  
--- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala 
---
@@ -1051,11 +1058,13 @@ private[spark] class BlockManager(
   }
   blockIsUpdated = true
 }
+pendingToRemove.put(blockId, currentTaskAttemptId)
--- End diff --

This put should happen as soon as we check `containsKey` in L1035. Between 
now and then many things could happen; another thread may find that 
`containsKey` is false at the same time and both threads can still end up 
trying to drop the block.


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-04 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

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


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-04 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217060468
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-04 Thread SparkQA
Github user SparkQA commented on the pull request:

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


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-04 Thread andrewor14
Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/11546#discussion_r62139309
  
--- Diff: core/src/main/scala/org/apache/spark/executor/Executor.scala ---
@@ -227,6 +228,17 @@ private[spark] class Executor(
   logError(errMsg)
 }
   }
+
+  if (releasedLocks.nonEmpty) {
+val errMsg =
+  s"${releasedLocks.size} block locks were not released by TID 
= $taskId:\n" +
--- End diff --

@markhamstra this is a little confusing but it's basically saying that all 
the locks are supposed to be released by now, so if we have to release any 
locks here (i.e. if `releasedLocks.nonEmpty`) then something it wrong.


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-04 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217047264
  
**[Test build #57824 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57824/consoleFull)**
 for PR 11546 at commit 
[`aaa6a96`](https://github.com/apache/spark/commit/aaa6a96704d9f1435965812a99d3ac37a61ac5ff).


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

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-05-04 Thread andrewor14
Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-217047021
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-04-19 Thread cenyuhai
Github user cenyuhai commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-212208592
  
@jamesecahill I don't know whether @JoshRosen will provide any other patch 
for this issue. But I have fixed this bug in my production environment by 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request: [SPARK-13566][CORE] Avoid deadlock between Blo...

2016-04-19 Thread jamesecahill
Github user jamesecahill commented on the pull request:

https://github.com/apache/spark/pull/11546#issuecomment-211921166
  
Any word on if this, or any other fix for SPARK-13566 will make it into 
1.6.2? 


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

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