attilapiros commented on a change in pull request #23540: [SPARK-26615][Core]
Fixing transport server/client resource leaks in the core unittests
URL: https://github.com/apache/spark/pull/23540#discussion_r247777099
##########
File path: core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala
##########
@@ -140,18 +138,8 @@ class BlockManagerSuite extends SparkFunSuite with
Matchers with BeforeAndAfterE
override def afterEach(): Unit = {
try {
conf = null
- if (store != null) {
- store.stop()
- store = null
- }
- if (store2 != null) {
- store2.stop()
- store2 = null
- }
- if (store3 != null) {
- store3.stop()
- store3 = null
- }
+ allStores.foreach(_.stop())
+ allStores.clear()
Review comment:
There are more leaks some are very well hidden :)
The not so pro at hiding (which can be easily corrected using the old
cleanup stategy) is
[BlockManagerSuite.scala#L1397](https://github.com/apache/spark/blob/master/core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala#L1397
)
And there are two `makeBlockManager` calls where exceptions are expected:
-
[BlockManagerSuite.scala#L1391](https://github.com/apache/spark/blob/master/core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala#L1391)
-
[BlockManagerSuite.scala#L1384](https://github.com/apache/spark/blob/master/core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala#L1384)
The exception is coming from
[`blockManager.initialize("app-id")`](https://github.com/apache/spark/blob/master/core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala#L109)
which goes further down to
[`registerWithExternalShuffleServer()`](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/storage/BlockManager.scala#L264).
But here we already have an initialised blockTransferService and a
shuffleClient. So `makeBlockManager` creates a BlockManager which should be
cleaned up but because of the exception that instance cannot be saved to the
old store variables. This is where the old cleanup strategy was broken.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
