srowen commented on a change in pull request #32534:
URL: https://github.com/apache/spark/pull/32534#discussion_r633552715
##########
File path: core/src/main/scala/org/apache/spark/storage/memory/MemoryStore.scala
##########
@@ -387,6 +388,14 @@ private[spark] class MemoryStore(
def remove(blockId: BlockId): Boolean = memoryManager.synchronized {
val entry = entries.synchronized {
+ entries.get(blockId) match {
Review comment:
Why would the objects be gone? remove just removes copies of references
from the map; they still exist and won't go away if you keep the references to
them here.
##########
File path: core/src/main/scala/org/apache/spark/storage/memory/MemoryStore.scala
##########
@@ -405,6 +427,23 @@ private[spark] class MemoryStore(
def clear(): Unit = memoryManager.synchronized {
entries.synchronized {
+ val entryManualCloseTasks = ArrayBuffer.empty[Future[Unit]]
+ entries.values.asScala.foreach {
+ case e: DeserializedMemoryEntry[_] => e.value.foreach {
+ case o: AutoCloseable =>
+ entryManualCloseTasks += Future {
Review comment:
Not bad; I am a little concerned about the extra complexity and which
thread pool this gets called in, maybe creating its own bottleneck. But you
could argue it's better than waiting in clear(). Are any of the objects closed
here expensive to close? I didn't check.
##########
File path: core/src/test/scala/org/apache/spark/storage/MemoryStoreSuite.scala
##########
@@ -546,4 +550,105 @@ class MemoryStoreSuite
}
}
}
+
+ test("put native objects to MemoryStore and remove") {
Review comment:
This seems quite complex - is there any way to test this more easily? If
it's this hard to test directly, I may say just punt on it. This is kind of
best-effort to close the objects anyway.
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
