Github user JeetKunDoug commented on a diff in the pull request:
https://github.com/apache/spark/pull/21322#discussion_r188325608
--- Diff:
core/src/main/scala/org/apache/spark/storage/memory/MemoryStore.scala ---
@@ -384,15 +385,36 @@ private[spark] class MemoryStore(
}
}
+ private def maybeReleaseResources(entry: MemoryEntry[_]): Unit = {
+ entry match {
+ case SerializedMemoryEntry(buffer, _, _) => buffer.dispose()
+ case DeserializedMemoryEntry(objs: Array[Any], _, _) =>
maybeCloseValues(objs)
--- End diff --
Actually, digging further, there's other places where we may deserialize an
object from the disk store and never put it into the memory store - it seems
like punting on a guarantee that your AutoClosable object is closed and making
this a best-effort thing when calling `BlockManager.removeBroadcast` (which is
how we used it in the case that caused us to put together this PR) may make the
most sense. It'll still be better than depending on GC and a finalizer to get
the resources cleaned up when the driver can call `Broadcast#destroy` but we
can document it as a best practice to have one just in case the close() call
doesn't happen due to edge cases.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
