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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
