Github user wgtmac commented on a diff in the pull request:
https://github.com/apache/spark/pull/15248#discussion_r81007488
--- Diff:
core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
@@ -179,7 +180,11 @@ class HistoryServer(
}
--- End diff --
I disagree that getApplicationList() can be removed because SparkUI class
is still using this API though deleting it is just more pieces of code.
Yes, both original approach and view-based approach are suffering the
risking of reading data from a mutable data source. However, I took a look at
the code. The iterator (or the view) returned is from the mutable LinkedHashMap
**applications** of FsHistoryProvider. It is only changed via
FsHistoryProvider.mergeApplicationListing() by merging the original
applications map with newly-updated apps to a new LinkedHashMap and change the
applications reference to it. So the returned iterator or view is still binding
to the old LinkedHashMap applications which is safe. I may be wrong, can you
confirm?
I'll think about using the iterator returned from getApplicationList() as
you said in the last comment.
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
