[CELEBORN-2376] Filter empty ResourceConsumption entries in WorkerInfo toString output

[gaoyajun02]

What changes were proposed in this pull request?

Added an isEmpty() method to ResourceConsumption that returns true when all four counters (diskBytesWritten, diskFileCount, hdfsBytesWritten, hdfsFileCount) are zero and subResourceConsumptions is empty.

WorkerInfo.toString now uses this method to skip users with no active resource consumption, so that only entries with non-zero usage are included in the output. An additional guard handles the edge case where all entries are filtered out, falling back to "empty" rather than producing a blank string.

Why are the changes needed?

In clusters with many registered users, WorkerInfo.toString — surfaced in both HTTP API responses and log output — can include a large number of entries like:

UserIdentifier: tenant1.user1, ResourceConsumption(diskBytesWritten: 0 B, diskFileCount: 0, hdfsBytesWritten: 0 B, hdfsFileCount: 0, subResourceConsumptions: empty)

These zero-value entries carry no operational information and add noise that makes it harder to identify active workloads at a glance. Filtering them out keeps the output focused on users with actual resource usage.

Does this PR resolve a correctness bug?

• Yes

Does this PR introduce any user-facing change?

• Yes

WorkerInfo.toString output (returned by GET /api/v1/workers and visible in Worker logs) will no longer include users whose resource consumption is entirely zero.

How was this patch tested?

Covered by existing unit tests.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[codecov]

Codecov Report

❌ Patch coverage is 37.50000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.72%. Comparing base (3820244) to head (9e64b0d).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
...a/org/apache/celeborn/common/meta/WorkerInfo.scala	50.00%	1 Missing and 2 partials ⚠️
...he/celeborn/common/quota/ResourceConsumption.scala	0.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #3754      +/-   ##
============================================
+ Coverage     57.53%   57.72%   +0.19%     
  Complexity      214      214              
============================================
  Files           396      397       +1     
  Lines         27857    27884      +27     
  Branches       2710     2715       +5     
============================================
+ Hits          16025    16093      +68     
+ Misses        10682    10639      -43     
- Partials       1150     1152       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[SteNicholas]

@gaoyajun02, thanks for update. The changes are correct and low-risk. The isEmpty helper and the toString rewrite are null-safe (the map is a ConcurrentHashMap, so values can't be null), the .iterator.collect{...}.mkString("") is equivalent to the old .map aside from the intended filter, and the if (rendered.isEmpty) "empty" fallback correctly restores the sentinel when every entry is filtered. The rewrite also cleans up the old latently Any/Unit-typed else if branch.

Two non-blocking notes below (a consistency point and a test-coverage point) — neither is a correctness bug.

[SteNicholas]

Consistency / reuse: there's already an equivalent "filter out empty user resource consumption" predicate that feeds GET /api/v1/workers. ApiUtils.workerResourceConsumptions keeps an entry only when CollectionUtils.isNotEmpty(ur._2.subResourceConsumptions) (service/src/main/scala/.../http/api/v1/ApiUtils.scala:69-70, comment: // filter out user resource consumption with empty sub resource consumptions).

This new isEmpty uses a different rule — the four counters and the sub-map. For the shapes the worker snapshot actually produces the two agree (active users always carry a non-empty sub-map via StorageManager.userResourceConsumptionSnapshot; the zeroed placeholders inserted by WorkerInfo.updateThenGetUserResourceConsumption are all-zero with an empty sub-map). But they diverge for an entry with non-zero counters and an empty/null sub-map, and they are now two independent definitions of the same concept maintained separately.

Consider unifying: either have ApiUtils reuse this new isEmpty, or filter toString on the same isNotEmpty(subResourceConsumptions) predicate, so the text/log output and the JSON API stay in lockstep.

[SteNicholas]

Test coverage: the PR says "Covered by existing unit tests", but the existing WorkerInfoSuite "toString output" case doesn't actually exercise this new branch. worker4 renders a single non-zero entry, and worker1-worker3 have empty maps that hit the outer userResourceConsumption.isEmpty branch. Nothing covers:

• a zero entry (ResourceConsumption(0, 0, 0, 0)) being filtered out,
• a mix of zero + non-zero entries (only the non-zero one rendered),
• the if (rendered.isEmpty) "empty" fallback when every entry is filtered.

Worth adding a small case so a future regression that re-renders zero entries is caught.