yew1eb commented on PR #3858:
URL: https://github.com/apache/celeborn/pull/3858#issuecomment-6416172439
Thanks for the PR! One nit on config naming: the three new worker-side
config entries in `CelebornConf.scala` are registered under
`.categories("metrics")` but use the `celeborn.scale.*` prefix:
- `celeborn.scale.metricCollectorClassName`
- `celeborn.scale.metricInterval`
- `celeborn.scale.metricWindowSize`
All existing keys in the `metrics` category use the `celeborn.metrics.*`
prefix (e.g. `celeborn.metrics.enabled`,
`celeborn.metrics.worker.appLevel.enabled`,
`celeborn.metrics.timer.slidingWindow.size`). Since these are worker-side
metric configs, it would be more consistent to align the prefix, e.g.:
- `celeborn.metrics.scale.metricCollectorClassName`
- `celeborn.metrics.scale.metricInterval`
- `celeborn.metrics.scale.metricWindowSize`
(or `celeborn.metrics.worker.scale.*`, matching the other worker metric
keys). Wdyt?
--
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]
