Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/23088#discussion_r236092947
--- Diff: core/src/main/scala/org/apache/spark/status/AppStatusStore.scala
---
@@ -222,29 +223,20 @@ private[spark] class AppStatusStore(
val indices = quantiles.map { q => math.min((q * count).toLong, count
- 1) }
def scanTasks(index: String)(fn: TaskDataWrapper => Long):
IndexedSeq[Double] = {
- Utils.tryWithResource(
- store.view(classOf[TaskDataWrapper])
- .parent(stageKey)
- .index(index)
- .first(0L)
- .closeableIterator()
- ) { it =>
- var last = Double.NaN
- var currentIdx = -1L
- indices.map { idx =>
- if (idx == currentIdx) {
- last
- } else {
- val diff = idx - currentIdx
- currentIdx = idx
- if (it.skip(diff - 1)) {
- last = fn(it.next()).toDouble
- last
- } else {
- Double.NaN
- }
- }
- }.toIndexedSeq
+ val quantileTasks = store.view(classOf[TaskDataWrapper])
--- End diff --
I get it, but, it seems like there was a particular reason for using an
iterator and skipping, and I'm not sure we can just undo it. If this is only to
filter for "SUCCESS", that much seems easy enough in the original code with an
'if' statement somewhere, no? similar code above still uses an iterator.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
