Repository navigation
[CELEBORN-2469] Add worker stats collection and reporting for autoscaling - #3858
Kalvin2077 wants to merge 5 commits into
Conversation
|
@Kalvin2077, I do not agree with the current implementation. Have you verified the heartbeat load caused by adding worker metrics in a large-scale celeborn cluster? Meanwhile, the details of this FLIP have not yet been fully aligned. |
Thanks for the feedback. You're right. The current implementation introduces a typed WorkerStats schema, while CIP-13 still proposes a simple stats map inside WorkerStatus. I should have aligned the schema on the CIP thread before implementing it. To make sure I address the right risk: is the concern mainly the payload size of the typed schema, or the broader master-side and aggregate heartbeat cost at scale? |
16fc232 to
b3079e6
Compare
Co-Authored-By: Codex <noreply@openai.com> AI-Model: gpt-6.1-sol AI-Contributed/Feature: 114/114 AI-Contributed/UT: 302/302
Co-Authored-By: Codex <noreply@openai.com> AI-Model: gpt-6.1-sol AI-Contributed/Feature: 228/228 AI-Contributed/UT: 354/354
Co-Authored-By: Codex <noreply@openai.com> AI-Model: gpt-6.1-sol AI-Contributed/Feature: 3/3 AI-Contributed/UT: 0/0
b3079e6 to
89514c8
Compare
| val disks = worker.storageManager.localDisksSnapshot() | ||
| val usableDisks = disks.filter(_.status != DiskStatus.IO_HANG) | ||
| val usableBytes = usableDisks.map(_.actualUsableSpace.toDouble).sum | ||
| val totalBytes = disks.map(_.totalSpace.toDouble).sum |
There was a problem hiding this comment.
Could DiskUsedRatio use Celeborn's effective allocatable capacity as the denominator? StorageManager caps actualUsableSpace by configuredUsableSpace, but sets totalSpace to the filesystem's physical total. On an otherwise empty 1 TiB disk configured with capacity=100Gi, this reports about 90% used even though Celeborn has used none of its configured capacity. This would give autoscaling an incorrect disk pressure signal. Please use a consistent capacity basis for the numerator and denominator, preserve the physical free space and reserve limit, and add a test where capacity= is smaller than the physical disk.
d3859cd to
cf21ba4
Compare
What changes were proposed in this pull request?
Implement the Worker metrics portion of CIP-13:
map<string, double> stats = 3inPbWorkerStatus.The numeric map is an explicit difference from CIP-13’s proposed string map and remains part of the design discussion.
Why are the changes needed?
Future scaling policies need Worker resource observations. This PR supplies the collection and reporting path; scaling decisions and platform operations are follow-up work.
Does this PR resolve a correctness bug?
Does this PR introduce any user-facing change?
Adds
celeborn.scale.metricCollectorClassName,celeborn.scale.metricInterval(default5s) andceleborn.scale.metricWindowSize(default6).How was this patch tested?
21 selected tests passed on JDK 17, covering protobuf compatibility, collection, cache cleanup, failure isolation and Worker-to-Master RPCs. Module builds, Spotless and Apache Rat passed.
Performance impact
A local TCP benchmark compared heartbeats without metrics against the four default metrics. Each case ran twice for 10 seconds after warmup, at
workerCount / 30heartbeats/s. All 14,672 measured responses in this comparison succeeded.The four metrics add 128 protobuf bytes per heartbeat, approximately 42.67 kB/s at 10,000 Workers with a 30-second period, excluding transport framing. Heartbeats read cached measurements; collection adds no extra RPC.
These are observed differences between two-run averages. CPU ranges overlapped. Client and non-HA Master shared one JVM, and the short runs did not cover a full heartbeat cycle or measure collector CPU. They do not establish physical-cluster capacity; representative cross-host and HA validation is still needed.
Possible follow-up PRs
ScaleManagerthrough aregisterWorkerStatscallback.