You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/04/21 20:01:17 UTC
[GitHub] [spark] hvanhovell opened a new pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
hvanhovell opened a new pull request #28286:
URL: https://github.com/apache/spark/pull/28286
### What changes were proposed in this pull request?
This PR increases the thread safety of the `BytesToBytesMap`:
- It makes the `iterator()` and `destructiveIterator()` methods used their own `Location` object. This used to be shared, and this was causing issues when the map was being iterated over in two threads by two different iterators.
- Removes the `safeIterator()` function. This is not needed anymore.
- Improves the documentation of a couple of methods w.r.t. thread-safety.
### Why are the changes needed?
It is unexpected an iterator shares the object it is returning with all other iterators. This is a violation of the iterator contract, and it causes issues with iterators over a map that are consumed in different threads.
### Does this PR introduce any user-facing change?
No
### How was this patch tested?
Existing tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-617384087
**[Test build #121595 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121595/testReport)** for PR 28286 at commit [`c4d6c7b`](https://github.com/apache/spark/commit/c4d6c7b6575e9ea9a34b56431e4a181ec020e1b8).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-617460461
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun edited a comment on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-683154948
BTW, @cxzl25 . Since it's a correctness bug report, +1 for backporting.
However, in order to backport `branch-3.0` and `branch-2.4`, if possible, we need a valid test case for your reporting.
SPARK-31511 was an improvement PR and only was merged for 3.1.0.
cc @gatorsmile
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-683153656
Thank you for sharing, @cxzl25
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-617384641
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu commented on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu commented on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-685424888
@cxzl25 Can you show what kind of production environment and use case did you have this problem
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-682022369
Thanks for reporting it! I think we should backport this patch according to the bug report from the real world.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-617384087
**[Test build #121595 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121595/testReport)** for PR 28286 at commit [`c4d6c7b`](https://github.com/apache/spark/commit/c4d6c7b6575e9ea9a34b56431e4a181ec020e1b8).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-683159760
Also, cc @zhengruifeng for 3.0.1 and @ScrapCodes for 2.4.7
Since there is no Jira issue which is marked as `Blocker`, I guess this is up to the release manager's decision.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] hvanhovell commented on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
hvanhovell commented on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-683466734
@dongjoon-hyun let's just backport it. This is tricky to test, you need multiple consumers of a hash map in different threads that have to go through a fairly large amount of data, and then you **might** catch it. A test for this is likely to test nothing, and be fairly expensive.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-683154948
BTW, @cxzl25 . Since it's a correctness bug report, +1 for backporting.
However, in order to backport `branch-3.0` and `branch-2.4`, we need a valid test case for your reporting.
SPARK-31511 was an improvement PR and only was merged for 3.1.0.
cc @gatorsmile
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-683473781
Got it, @hvanhovell . Okay, if you think so.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-683474244
I added a new fixed version `3.0.2` to SPARK-31511 because `3.0.1` RC3 vote is still alive.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-617459862
**[Test build #121595 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121595/testReport)** for PR 28286 at commit [`c4d6c7b`](https://github.com/apache/spark/commit/c4d6c7b6575e9ea9a34b56431e4a181ec020e1b8).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-683882946
Thank you so much, @cxzl25
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-683473979
This is backported to `branch-3.0`. Could you make a backporting PR for `branch-2.4` because there is a conflict?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on issue #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on issue #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-617631415
LGTM too
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on issue #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-617438255
Thanks, @hvanhovell . The proposed change looks reasonable to me. Is this PR for Apache Spark 3.1.0 as an improvement?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-617460461
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-683155224
@hvanhovell . Could you switch SPARK-31511 from `Improvement` to `Bug` and label `correctness`, please?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cxzl25 commented on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-683543986
If you want to use sql to reproduce this bug, it is really difficult.
I write a ut to test UnsafeHashedRelation.
I'm not sure if this can help.
org.apache.spark.sql.execution.joins.HashedRelationSuite
```scala
test("SPARK-31511: Make BytesToBytesMap iterators thread-safe") {
val ser = sparkContext.env.serializer.newInstance()
val key = Seq(BoundReference(0, LongType, false))
val unsafeProj = UnsafeProjection.create(
Seq(BoundReference(0, LongType, false), BoundReference(1, IntegerType, true)))
val rows = (0 until 10000).map(i => unsafeProj(InternalRow(Int.int2long(i), i + 1)).copy())
val unsafeHashed = UnsafeHashedRelation(rows.iterator, key, 1, mm)
val os = new ByteArrayOutputStream()
val thread1 = new Thread {
override def run(): Unit = {
val out = new ObjectOutputStream(os)
unsafeHashed.asInstanceOf[UnsafeHashedRelation].writeExternal(out)
out.flush()
}
}
val thread2 = new Thread {
override def run(): Unit = {
val threadOut = new ObjectOutputStream(new ByteArrayOutputStream())
unsafeHashed.asInstanceOf[UnsafeHashedRelation].writeExternal(threadOut)
threadOut.flush()
}
}
thread1.start()
thread2.start()
thread1.join()
thread2.join()
val unsafeHashed2 = ser.deserialize[UnsafeHashedRelation](ser.serialize(unsafeHashed))
val os2 = new ByteArrayOutputStream()
val out2 = new ObjectOutputStream(os2)
unsafeHashed2.writeExternal(out2)
out2.flush()
assert(java.util.Arrays.equals(os.toByteArray, os2.toByteArray))
}
```
before:
after:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-617384641
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-683894939
@cxzl25 can you open a PR to add this test? You can also help with the branch 2.4 backport if you have time. Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AngersZhuuuu removed a comment on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
AngersZhuuuu removed a comment on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-685424888
@cxzl25 Can you show what kind of production environment and use case did you have this problem
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cxzl25 edited a comment on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
cxzl25 edited a comment on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-682019408
I encountered a problem with incorrect results in the production environment. After several days of research, I found that the patch can solve the problem.
**It is recommended to fix it in the previous version, because before this patch, the result may be incorrect.**
@HyukjinKwon @dongjoon-hyun
Before this patch, `BytesToBytesMap.iterator` was not thread-safe.
Because `BroadcastExchangeExec` is multi-threaded,
Thread 1 calls `BytesToBytesMap.iterator` in broadcast `UnsafeHashedRelation`.
Thread 2 needs to write the `UnsafeHashedRelation` of thread 1 to disk because of insufficient driver memory, and calls `BytesToBytesMap.iterator`.
https://github.com/apache/spark/blob/8749b2b6fae5ee0ce7b48aae6d859ed71e98491d/sql/core/src/main/scala/org/apache/spark/sql/execution/joins/HashedRelation.scala#L368
In addition, we can check whether numKeys numValues is consistent with the numKeys numValues of the constructed `BytesToBytesMap`
https://github.com/apache/spark/blob/8749b2b6fae5ee0ce7b48aae6d859ed71e98491d/sql/core/src/main/scala/org/apache/spark/sql/execution/joins/HashedRelation.scala#L383-L390
Stack trace output in iterator
broadcast-exchange-0
2020-08-27 23:06:00.270000
BytesToBytesMap Addr:1328576284
```
java.lang.Thread.getStackTrace(Thread.java:1556)
org.apache.spark.unsafe.map.BytesToBytesMap.iterator(BytesToBytesMap.java:418)
org.apache.spark.sql.execution.joins.UnsafeHashedRelation.org$apache$spark$sql$execution$joins$UnsafeHashedRelation$$write(HashedRelation.scala:225)
...
org.apache.spark.broadcast.TorrentBroadcast$.blockifyObject(TorrentBroadcast.scala:277)
org.apache.spark.broadcast.TorrentBroadcast.writeBlocks(TorrentBroadcast.scala:126)
org.apache.spark.broadcast.TorrentBroadcast.<init>(TorrentBroadcast.scala:88)
org.apache.spark.broadcast.TorrentBroadcastFactory.newBroadcast(TorrentBroadcastFactory.scala:34)
org.apache.spark.broadcast.BroadcastManager.newBroadcast(BroadcastManager.scala:56)
org.apache.spark.SparkContext.broadcast(SparkContext.scala:1500)
```
broadcast-exchange-1
2020-08-27 23:06:00.429000
BytesToBytesMap Addr:1328576284
```
java.lang.Thread.getStackTrace(Thread.java:1556)
org.apache.spark.unsafe.map.BytesToBytesMap.iterator(BytesToBytesMap.java:418)
org.apache.spark.sql.execution.joins.UnsafeHashedRelation.org$apache$spark$sql$execution$joins$UnsafeHashedRelation$$write(HashedRelation.scala:225)
org.apache.spark.sql.execution.joins.UnsafeHashedRelation$$anonfun$write$1.apply$mcV$sp(HashedRelation.scala:198)
...
org.apache.spark.storage.DiskStore.put(DiskStore.scala:69)
org.apache.spark.storage.BlockManager.dropFromMemory(BlockManager.scala:1378)
org.apache.spark.storage.memory.MemoryStore.org$apache$spark$storage$memory$MemoryStore$$dropBlock$1(MemoryStore.scala:524)
org.apache.spark.storage.memory.MemoryStore$$anonfun$evictBlocksToFreeSpace$2.apply(MemoryStore.scala:545)
org.apache.spark.storage.memory.MemoryStore$$anonfun$evictBlocksToFreeSpace$2.apply(MemoryStore.scala:539)
scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:48)
org.apache.spark.storage.memory.MemoryStore.evictBlocksToFreeSpace(MemoryStore.scala:539)
org.apache.spark.memory.StorageMemoryPool.acquireMemory(StorageMemoryPool.scala:92)
org.apache.spark.memory.StorageMemoryPool.acquireMemory(StorageMemoryPool.scala:73)
org.apache.spark.memory.UnifiedMemoryManager.acquireStorageMemory(UnifiedMemoryManager.scala:179)
org.apache.spark.memory.UnifiedMemoryManager.acquireUnrollMemory(UnifiedMemoryManager.scala:186)
org.apache.spark.storage.memory.MemoryStore.reserveUnrollMemoryForThisTask(MemoryStore.scala:582)
org.apache.spark.storage.memory.MemoryStore.putIteratorAsValues(MemoryStore.scala:223)
org.apache.spark.storage.BlockManager$$anonfun$doPutIterator$1.apply(BlockManager.scala:1039)
org.apache.spark.storage.BlockManager$$anonfun$doPutIterator$1.apply(BlockManager.scala:1030)
org.apache.spark.storage.BlockManager.doPut(BlockManager.scala:970)
org.apache.spark.storage.BlockManager.doPutIterator(BlockManager.scala:1030)
org.apache.spark.storage.BlockManager.putIterator(BlockManager.scala:793)
org.apache.spark.storage.BlockManager.putSingle(BlockManager.scala:1351)
org.apache.spark.broadcast.TorrentBroadcast.writeBlocks(TorrentBroadcast.scala:122)
org.apache.spark.broadcast.TorrentBroadcast.<init>(TorrentBroadcast.scala:88)
org.apache.spark.broadcast.TorrentBroadcastFactory.newBroadcast(TorrentBroadcastFactory.scala:34)
org.apache.spark.broadcast.BroadcastManager.newBroadcast(BroadcastManager.scala:56)
org.apache.spark.SparkContext.broadcast(SparkContext.scala:1500)
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cxzl25 commented on pull request #28286: [SPARK-31511][SQL] Make BytesToBytesMap iterators thread-safe
Posted by GitBox <gi...@apache.org>.
cxzl25 commented on pull request #28286:
URL: https://github.com/apache/spark/pull/28286#issuecomment-682019408
I encountered a problem with incorrect results in the production environment. After several days of research, I found that the patch can solve the problem.
**It is recommended to fix it in the previous version, because before this patch, the result may be incorrect.**
@HyukjinKwon @dongjoon-hyun
Before this patch, `BytesToBytesMap.iterator` was not thread-safe.
Because `BroadcastExchangeExec` is multi-threaded,
Thread 1 calls `BytesToBytesMap.iterator` in broadcast `UnsafeHashedRelation`.
Thread 2 needs to write the `UnsafeHashedRelation` of thread 1 to disk because of insufficient driver memory, and calls `BytesToBytesMap.iterator`.
https://github.com/apache/spark/blob/8749b2b6fae5ee0ce7b48aae6d859ed71e98491d/sql/core/src/main/scala/org/apache/spark/sql/execution/joins/HashedRelation.scala#L368
broadcast-exchange-0
2020-08-27 23:06:00.270000
BytesToBytesMap Addr:1328576284
```
java.lang.Thread.getStackTrace(Thread.java:1556)
org.apache.spark.unsafe.map.BytesToBytesMap.iterator(BytesToBytesMap.java:418)
org.apache.spark.sql.execution.joins.UnsafeHashedRelation.org$apache$spark$sql$execution$joins$UnsafeHashedRelation$$write(HashedRelation.scala:225)
...
org.apache.spark.broadcast.TorrentBroadcast$.blockifyObject(TorrentBroadcast.scala:277)
org.apache.spark.broadcast.TorrentBroadcast.writeBlocks(TorrentBroadcast.scala:126)
org.apache.spark.broadcast.TorrentBroadcast.<init>(TorrentBroadcast.scala:88)
org.apache.spark.broadcast.TorrentBroadcastFactory.newBroadcast(TorrentBroadcastFactory.scala:34)
org.apache.spark.broadcast.BroadcastManager.newBroadcast(BroadcastManager.scala:56)
org.apache.spark.SparkContext.broadcast(SparkContext.scala:1500)
```
broadcast-exchange-1
2020-08-27 23:06:00.429000
BytesToBytesMap Addr:1328576284
```
java.lang.Thread.getStackTrace(Thread.java:1556)org.apache.spark.unsafe.map.BytesToBytesMap.iterator(BytesToBytesMap.java:418) org.apache.spark.sql.execution.joins.UnsafeHashedRelation.org$apache$spark$sql$execution$joins$UnsafeHashedRelation$$write(HashedRelation.scala:225)
org.apache.spark.sql.execution.joins.UnsafeHashedRelation$$anonfun$write$1.apply$mcV$sp(HashedRelation.scala:198)
...
org.apache.spark.storage.DiskStore.put(DiskStore.scala:69)
org.apache.spark.storage.BlockManager.dropFromMemory(BlockManager.scala:1378)
org.apache.spark.storage.memory.MemoryStore.org$apache$spark$storage$memory$MemoryStore$$dropBlock$1(MemoryStore.scala:524)
org.apache.spark.storage.memory.MemoryStore$$anonfun$evictBlocksToFreeSpace$2.apply(MemoryStore.scala:545)
org.apache.spark.storage.memory.MemoryStore$$anonfun$evictBlocksToFreeSpace$2.apply(MemoryStore.scala:539)
scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:48)
org.apache.spark.storage.memory.MemoryStore.evictBlocksToFreeSpace(MemoryStore.scala:539)
org.apache.spark.memory.StorageMemoryPool.acquireMemory(StorageMemoryPool.scala:92)
org.apache.spark.memory.StorageMemoryPool.acquireMemory(StorageMemoryPool.scala:73)
org.apache.spark.memory.UnifiedMemoryManager.acquireStorageMemory(UnifiedMemoryManager.scala:179)
org.apache.spark.memory.UnifiedMemoryManager.acquireUnrollMemory(UnifiedMemoryManager.scala:186)
org.apache.spark.storage.memory.MemoryStore.reserveUnrollMemoryForThisTask(MemoryStore.scala:582)
org.apache.spark.storage.memory.MemoryStore.putIteratorAsValues(MemoryStore.scala:223)
org.apache.spark.storage.BlockManager$$anonfun$doPutIterator$1.apply(BlockManager.scala:1039)
org.apache.spark.storage.BlockManager$$anonfun$doPutIterator$1.apply(BlockManager.scala:1030)
org.apache.spark.storage.BlockManager.doPut(BlockManager.scala:970)
org.apache.spark.storage.BlockManager.doPutIterator(BlockManager.scala:1030)
org.apache.spark.storage.BlockManager.putIterator(BlockManager.scala:793)
org.apache.spark.storage.BlockManager.putSingle(BlockManager.scala:1351)
org.apache.spark.broadcast.TorrentBroadcast.writeBlocks(TorrentBroadcast.scala:122)
org.apache.spark.broadcast.TorrentBroadcast.<init>(TorrentBroadcast.scala:88)
org.apache.spark.broadcast.TorrentBroadcastFactory.newBroadcast(TorrentBroadcastFactory.scala:34)
org.apache.spark.broadcast.BroadcastManager.newBroadcast(BroadcastManager.scala:56)
org.apache.spark.SparkContext.broadcast(SparkContext.scala:1500)
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org