[CELEBORN-1520] Minor logging fix for AppDiskUsageMetric and Fixed UTs
### What changes were proposed in this pull request? - Minor logging change in AppDiskUsageMetric which are not critical but slightly bothering. - Fixed UT's for AppDiskUsageMetric ### Why are the changes needed? 1. Current AppDiskUsageMetric UTs were like placeholder and just printing the output. They were not testing/verifying anything. 2. Minor logging change with AppDiskUsageMetric. - Comma formating was wrong ``` Snapshot start 2024-07-24T08:47:12.496 end 2024-07-24T08:57:12.497 Application application_1717149813731_19042841_2 used approximate 15.9 GiB ,Application application_1717149813731_19042841_1 used approximate 13.9 GiB ``` - We were printing an extra empty line after each summary. ``` 211:20:24.339 [master-app-disk-usage-metrics-logger] INFO org.apache.celeborn.common.meta.AppDiskUsageMetric - App Disk Usage Top50 Report Snapshot start 2024-07-24T09:17:12.498 end 2024-07-24T09:27:12.498 Application application_XXX used approximate 14.5 GiB Snapshot start 2024-07-24T08:17:12.495 end 2024-07-24T08:27:12.496 Application application_XXX used approximate 15.9 GiB 11:27:12.507 [master-app-disk-usage-metrics-logger] INFO org.apache.celeborn.common.meta.AppDiskUsageMetric - App Disk Usage Top50 Report Snapshot start 2024-07-24T09:17:12.498 end 2024-07-24T09:27:12.498 Application application_XXX used approximate 14.5 GiB Snapshot start 2024-07-24T08:17:12.495 end 2024-07-24T08:27:12.496 Application application_XXX used approximate 15.9 GiB ``` ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Fixed current UTs and verified from the logs. Closes #2643 from s0nskar/app_disk_usage. Authored-by: Sanskar Modi <sanskarmodi97@gmail.com> Signed-off-by: Cheng Pan <chengpan@apache.org>
This commit is contained in:
parent
e92d59ace5
commit
0a68ae049c
@ -30,7 +30,7 @@ import org.apache.celeborn.common.util.{ThreadUtils, Utils}
|
||||
|
||||
case class AppDiskUsage(var appId: String, var estimatedUsage: Long) {
|
||||
override def toString: String =
|
||||
s"Application $appId used approximate ${Utils.bytesToString(estimatedUsage)} "
|
||||
s"Application $appId used approximate ${Utils.bytesToString(estimatedUsage)}"
|
||||
}
|
||||
|
||||
class AppDiskUsageSnapShot(val topItemCount: Int) extends Logging with Serializable {
|
||||
@ -107,7 +107,7 @@ class AppDiskUsageSnapShot(val topItemCount: Int) extends Logging with Serializa
|
||||
s"Snapshot " +
|
||||
s"start ${LocalDateTime.ofInstant(Instant.ofEpochMilli(startSnapShotTime), zoneId)} " +
|
||||
s"end ${LocalDateTime.ofInstant(Instant.ofEpochMilli(endSnapShotTime), zoneId)}" +
|
||||
s" ${topNItems.filter(_ != null).mkString(",")}"
|
||||
s" ${topNItems.filter(_ != null).mkString(", ")}"
|
||||
}
|
||||
}
|
||||
|
||||
@ -143,13 +143,8 @@ class AppDiskUsageMetric(conf: CelebornConf) extends Logging {
|
||||
currentSnapShot.get().commit()
|
||||
}
|
||||
currentSnapShot.set(getNewSnapShot())
|
||||
val summaryStr = Some(summary()).map(str =>
|
||||
if (str != null && str.nonEmpty) {
|
||||
"\n" + str
|
||||
} else {
|
||||
str
|
||||
}).getOrElse("")
|
||||
logInfo(s"App Disk Usage Top$usageCount Report $summaryStr")
|
||||
val summaryStr = Some(summary()).getOrElse("")
|
||||
logInfo(s"App Disk Usage Top$usageCount Report: $summaryStr")
|
||||
}
|
||||
},
|
||||
60,
|
||||
@ -168,8 +163,8 @@ class AppDiskUsageMetric(conf: CelebornConf) extends Logging {
|
||||
val stringBuilder = new StringBuilder()
|
||||
for (i <- 0 until snapshotCount) {
|
||||
if (snapShots(i) != null && snapShots(i).topNItems.exists(_ != null)) {
|
||||
stringBuilder.append("\n")
|
||||
stringBuilder.append(snapShots(i))
|
||||
stringBuilder.append(" \n")
|
||||
}
|
||||
}
|
||||
stringBuilder.toString()
|
||||
|
||||
@ -36,26 +36,47 @@ class AppDiskUsageMetricSuite extends AnyFunSuite
|
||||
val WORKER2 = new WorkerInfo("host2", 211, 212, 213, 214, 215)
|
||||
val WORKER3 = new WorkerInfo("host3", 311, 312, 313, 314, 315)
|
||||
|
||||
def verifySnapShotOutput(snapShot: AppDiskUsageSnapShot, capacity: Int, appCount: Int): Unit = {
|
||||
val topNItemsEstimatedUsage = snapShot.topNItems
|
||||
.filter(usage => usage != null)
|
||||
.map(_.estimatedUsage)
|
||||
|
||||
assert(snapShot.topItemCount == capacity)
|
||||
assert(topNItemsEstimatedUsage.length == appCount)
|
||||
assert(topNItemsEstimatedUsage sameElements topNItemsEstimatedUsage.sorted.reverse)
|
||||
}
|
||||
|
||||
test("test snapshot ordering") {
|
||||
val snapShot = new AppDiskUsageSnapShot(50)
|
||||
val rand = new Random()
|
||||
for (i <- 1 to 5) {
|
||||
snapShot.updateAppDiskUsage(s"app-${i}", rand.nextInt(100000000) + 1)
|
||||
}
|
||||
|
||||
verifySnapShotOutput(snapShot, 50, 5)
|
||||
}
|
||||
|
||||
test("test snapshot ordering with capacity") {
|
||||
val snapShot = new AppDiskUsageSnapShot(50)
|
||||
val rand = new Random()
|
||||
for (i <- 1 to 60) {
|
||||
snapShot.updateAppDiskUsage(s"app-${i}", rand.nextInt(100000000) + 1)
|
||||
}
|
||||
println(snapShot.toString)
|
||||
|
||||
verifySnapShotOutput(snapShot, 50, 50)
|
||||
}
|
||||
|
||||
test("test snapshot ordering with duplicate entries") {
|
||||
val snapShot = new AppDiskUsageSnapShot(50)
|
||||
val rand = new Random()
|
||||
for (i <- 1 to 60) {
|
||||
for (i <- 1 to 10) {
|
||||
snapShot.updateAppDiskUsage(s"app-${i}", rand.nextInt(100000000) + 1)
|
||||
}
|
||||
for (i <- 1 to 15) {
|
||||
for (i <- 1 to 10) {
|
||||
snapShot.updateAppDiskUsage(s"app-${i}", rand.nextInt(100000000) + 1000000000)
|
||||
}
|
||||
|
||||
println(snapShot.toString)
|
||||
verifySnapShotOutput(snapShot, 50, 10)
|
||||
}
|
||||
|
||||
test("test app usage snapshot") {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user