Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4370#discussion_r24232333
--- Diff:
core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala ---
@@ -230,6 +230,17 @@ private[history] class FsHistoryProvider(conf:
SparkConf) extends ApplicationHis
}
/**
+ * Comparison function that defines the sort order for the application
listing.
+ *
+ * @return Whether `i1` should precede `i2`.
+ */
+ private def compareAppInfo(
+ i1: FsApplicationHistoryInfo,
+ i2: FsApplicationHistoryInfo): Boolean = {
+ i1.endTime >= i2.endTime && i1.startTime >= i2.startTime
--- End diff --
The rest LGTM, but, isn't this defining a different and inconsistent sort
order? For example if the first argument's start and end time are after and
before the other's start and times, respectively, then the method returns false
both ways, when one has to be true. The existing logic is to sort by endTime
descending, then startTime descending, so I think it's more like...
```
if (i1.endTime != i2.endTime) {
i1.endTime > i2.endTime
} else {
i1.startTime > i2.startTime
}
```
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
