[GitHub] [spark] srowen commented on a change in pull request #28769: [SPARK-31929][WEBUI] Close leveldbiterator when leveldb.close

[GitBox <gi...@apache.org>]

srowen commented on a change in pull request #28769:
URL: https://github.com/apache/spark/pull/28769#discussion_r438366689



##########
File path: core/src/main/scala/org/apache/spark/status/AppStatusStore.scala
##########
@@ -39,7 +39,13 @@ private[spark] class AppStatusStore(
   def applicationInfo(): v1.ApplicationInfo = {
     try {
       // The ApplicationInfo may not be available when Spark is starting up.
-      store.view(classOf[ApplicationInfoWrapper]).max(1).iterator().next().info
+      Utils.tryWithResource(

Review comment:
       An Iterator shouldn't be Iterable, really... hm, what about explicitly draining the iterator to a Seq in the methods that call asScala, so we can manage the iterator lifecycle? 
   
   Assuming there isn't much data there, materializing it could be fine. Or expose some other explicit method that returns the results from KVStore as a Seq, rather than just an iterator, if necessary.
   
   I still don't quite see how the iterator doesn't get closed; surely the .asScala wrapper has to evaluate all elements in the underlying iterator to produce the final filtered/mapped Seq. Hm. Maybe just being more explicit is better




----------------------------------------------------------------
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