vanzin commented on a change in pull request #26508: [SPARK-26260][Core]For
disk store tasks summary table should show only successful tasks summary
URL: https://github.com/apache/spark/pull/26508#discussion_r349333198
##########
File path:
core/src/test/scala/org/apache/spark/status/AppStatusStoreSuite.scala
##########
@@ -76,34 +79,47 @@ class AppStatusStoreSuite extends SparkFunSuite {
assert(store.count(classOf[CachedQuantile]) === 2)
}
- private def createLiveStore(inMemoryStore: InMemoryStore): AppStatusStore = {
+ private def createAppStore(store: KVStore, live: Boolean = false):
AppStatusStore = {
val conf = new SparkConf()
- val store = new ElementTrackingStore(inMemoryStore, conf)
- val listener = new AppStatusListener(store, conf, true, None)
- new AppStatusStore(store, listener = Some(listener))
- }
-
- test("SPARK-28638: only successful tasks have taskSummary when with in
memory kvstore") {
- val store = new InMemoryStore()
- (0 until 5).foreach { i => store.write(newTaskData(i, status = "FAILED")) }
- Seq(new AppStatusStore(store), createLiveStore(store)).foreach { appStore
=>
- val summary = appStore.taskSummary(stageId, attemptId, uiQuantiles)
- assert(summary.size === 0)
+ if (live) {
+ AppStatusStore.createLiveStore(conf)
+ } else {
+ new AppStatusStore(store)
}
}
- test("SPARK-28638: summary should contain successful tasks only when with in
memory kvstore") {
- val store = new InMemoryStore()
-
- for (i <- 0 to 5) {
- if (i % 2 == 1) {
- store.write(newTaskData(i, status = "FAILED"))
- } else {
- store.write(newTaskData(i))
+ test("SPARK-26260: task summary should contain only successful tasks'
metrics") {
Review comment:
This test and the next could use some refactoring. If one of them fails,
it's hard to know exactly what is failing.
So switch it around:
```
Seq(
("disk" -> createStore(disk = true, live = false),
("in memory" -> createStore(disk = false, live = false),
("in memory live" -> createStore(disk = false, live = true)
).foreach { case (hint, store) =>
test("SPARK-XXXXX: blah ($hint)") {
// test code
}
}
```
Temp dirs are cleaned up automatically. No need to worry.
Also you can probably merge the two tests if you check the task summary
before recording any successful tasks.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
