shellfish007 opened a new pull request, #3766: URL: https://github.com/apache/celeborn/pull/3766
### What changes were proposed in this pull request? Adds first-class S3 accounting to `ResourceConsumption`, mirroring the existing disk/hdfs pattern: - **`PbResourceConsumption`** — new fields `s3BytesWritten = 6`, `s3FileCount = 7` (backward compatible; older peers default to 0). - **`ResourceConsumption`** — new `s3BytesWritten`/`s3FileCount` fields, defaulted to 0 so all existing construction sites compile unchanged; included in `add`/`subtract`/`toString`/`simpleString`. - **`StorageManager.resourceConsumption()`** — tallies `fileInfos.filter(_.isS3)` into the S3 fields. - **`Worker`** — emits `s3FileCount`/`s3BytesWritten` gauges (and removes them on app cleanup) when `hasS3Storage`; top-N app ranking now sums `disk + hdfs + s3` bytes. - **`ResourceConsumptionSource`** — `S3_FILE_COUNT`/`S3_BYTES_WRITTEN` metric names. - **`PbSerDeUtils`**/**`MetaUtil`** — round-trip the new fields. - Tests updated (`PbSerDeUtilsTest` now covers S3 round-trip; `WorkerInfoSuite` toString; `MasterStateMachineSuiteJ`). OSS has the identical gap but is intentionally out of scope here. ### Why are the changes needed? Worker per-user and per-application `ResourceConsumption` metrics silently drop S3-backed shuffle files, so `diskBytesWritten`/`hdfsBytesWritten` (and their file counts) read 0 on an S3-only worker, and the top-N application ranking never sees S3 apps. Root cause — `StorageManager.resourceConsumption()`: ```scala val diskFileInfos = fileInfos.filter(!_.isDFS) // local disk val hdfsFileInfos = fileInfos.filter(_.isHdfs) // HDFS only ``` `DiskFileInfo.isDFS() = HDFS || S3 || OSS` and `isHdfs() = HDFS`, so an S3 file (`isDFS=true`, `isHdfs=false`) is excluded from both buckets. `isS3()` exists and is used throughout the storage I/O path but never in consumption accounting, and `ResourceConsumption` has no S3 field. This affects both `celeborn.metrics.worker.app.topResourceConsumption.count` (per-app) and `celeborn.master.userResourceConsumption.metrics.enabled` (per-user/tenant) — both read 0 on S3 storage. ### Does this PR introduce _any_ user-facing change? Yes — adds new worker gauges `s3FileCount`/`s3BytesWritten` and includes S3 bytes in top-N app resource consumption ranking. New proto fields are additive (unset ⇒ 0), so mixed-version worker/master heartbeats are safe. ### How was this patch tested? `PbSerDeUtilsTest` covers S3 round-trip serialization; `WorkerInfoSuite` and `MasterStateMachineSuiteJ` updated for the new fields. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
