apache / celeborn

Apache Celeborn is an elastic and high-performance service for shuffle and spilled data.
https://celeborn.apache.org/
Apache License 2.0
863 stars 351 forks source link

[CELEBORN-1520] Minor logging fix for AppDiskUsageMetric and Fixed UTs #2643

Closed s0nskar closed 1 month ago

s0nskar commented 1 month ago

What changes were proposed in this pull request?

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
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.

pan3793 commented 1 month ago

Thanks, merged to main/0.5/0.4

s0nskar commented 1 month ago

Thanks @pan3793 for the review.