refactor: enforce 1 tierAlias mapping per distinct tier. add tierAlias dim to some metrics#19595
Conversation
|
@jtuglu1 can you comment on if I am wrong in assuming that tiers should never belong to more than one alias? It is an assumption I made and created enforcement for because it made the metrics story more straight forward. But I don't want to clobber your initial intentions without discussion |
|
I think this seems reasonable to me – we currently are only using it in a way where 1 physical tier is aliased to at most 1 other virtual tier. It's worth considering whether we'd want to support multiple aliases (e.g. 1 physical tier being union of 2+ virtual tiers), but I think it's a fine assertion to make for now. |
| throw InvalidInput.exception( | ||
| "historicalTierAliases [%s] is invalid. Physical tier%s %s cannot belong to more than one alias.", | ||
| this.historicalTierAliases, | ||
| duplicateTiers.size() > 1 ? "s" : "", |
There was a problem hiding this comment.
nit: can we just do something like:
Physical tier(s) [%s] cannot...
FrankChen021
left a comment
There was a problem hiding this comment.
I have reviewed the code for correctness, edge cases, concurrency, and integration risks; no issues found.
Reviewed 10 of 10 changed files.
This is an automated review by Codex GPT-5.5
Description
1 associated alias enforcement
Explicitly enforce historical tiers belonging to a single alias.
Add tierAlias dimension to metrics
When using aliasing, it is helpful to have some associated tier based metrics report the alias the tier belongs to. This can make monitoring/alerting easier in clusters where you care more about the aggregate tier metrics than individual tiers.
Release note
Strictly forbid a historical tier from being associated with more than one tier alias. Add tierAlias dimension to some tiered metrics, allowing aggregation across aliases more easily on the monitoring/alerting side.
Key changed/added classes in this PR
CoordinatorDynamicConfigPrepareBalancerAndLoadQueuesThis PR has: