This is an automated email from the ASF dual-hosted git repository.
chengpan pushed a commit to branch branch-0.5
in repository https://gitbox.apache.org/repos/asf/celeborn.git
The following commit(s) were added to refs/heads/branch-0.5 by this push:
new 669f5a167 [CELEBORN-1520] Minor logging fix for AppDiskUsageMetric and
Fixed UTs
669f5a167 is described below
commit 669f5a167a9fb8dc1f0cc73a461cfccc3d02bef0
Author: Sanskar Modi <[email protected]>
AuthorDate: Thu Jul 25 13:47:12 2024 +0800
[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 <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
(cherry picked from commit 0a68ae049cc5de7b79af25fa6785dac47c799134)
Signed-off-by: Cheng Pan <[email protected]>
---
.../celeborn/common/meta/AppDiskUsageMetric.scala | 15 ++++-------
.../deploy/master/AppDiskUsageMetricSuite.scala | 29 +++++++++++++++++++---
2 files changed, 30 insertions(+), 14 deletions(-)
diff --git
a/common/src/main/scala/org/apache/celeborn/common/meta/AppDiskUsageMetric.scala
b/common/src/main/scala/org/apache/celeborn/common/meta/AppDiskUsageMetric.scala
index 92e08b5e0..45789f635 100644
---
a/common/src/main/scala/org/apache/celeborn/common/meta/AppDiskUsageMetric.scala
+++
b/common/src/main/scala/org/apache/celeborn/common/meta/AppDiskUsageMetric.scala
@@ -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()
diff --git
a/master/src/test/scala/org/apache/celeborn/service/deploy/master/AppDiskUsageMetricSuite.scala
b/master/src/test/scala/org/apache/celeborn/service/deploy/master/AppDiskUsageMetricSuite.scala
index c91d197d7..f0820dd43 100644
---
a/master/src/test/scala/org/apache/celeborn/service/deploy/master/AppDiskUsageMetricSuite.scala
+++
b/master/src/test/scala/org/apache/celeborn/service/deploy/master/AppDiskUsageMetricSuite.scala
@@ -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") {
