[fix](metric) Change partition near-limit metrics from counters to gauges#61845
[fix](metric) Change partition near-limit metrics from counters to gauges#61845dataroaring merged 2 commits intomasterfrom
Conversation
…uges The auto_partition_near_limit_count and dynamic_partition_near_limit_count metrics were LongCounterMetric (monotonically increasing) and never decreased, even when the near-limit condition resolved. Changed them to GaugeMetricImpl updated by TabletStatMgr's periodic table scan, so they reflect the current number of tables near the partition limit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
There was a problem hiding this comment.
Pull request overview
Updates FE partition near-limit metrics so they reflect the current number of tables approaching partition-count limits, rather than a monotonically increasing event count, by moving computation into the existing periodic TabletStatMgr scan.
Changes:
- Replaced
auto_partition_near_limit_count/dynamic_partition_near_limit_countfrom counters to gauges (names preserved). - Removed event-driven metric increments from
FrontendServiceImplandDynamicPartitionUtil. - Added near-limit table counting during
TabletStatMgr’s periodic all-table scan and publishes results toMetricRepogauges.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java | Removes counter increment on auto-partition near-limit warning. |
| fe/fe-core/src/main/java/org/apache/doris/metric/MetricRepo.java | Changes near-limit metrics to gauges while preserving metric names and registers them. |
| fe/fe-core/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java | Removes counter increment on dynamic-partition near-limit warning during property analysis. |
| fe/fe-core/src/main/java/org/apache/doris/catalog/TabletStatMgr.java | Computes and sets the new near-limit gauge values during periodic stats scan. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| List<Partition> allPartitions = olapTable.getAllPartitions(); | ||
| partitionCount += allPartitions.size(); | ||
| int tablePartitionNum = allPartitions.size(); | ||
| partitionCount += tablePartitionNum; | ||
| // Check if this table's partition count is near the limit (>80%) | ||
| if (olapTable.getPartitionInfo().enableAutomaticPartition()) { | ||
| int limit = Config.max_auto_partition_num; | ||
| if (tablePartitionNum > limit * 8L / 10) { |
There was a problem hiding this comment.
OlapTable.getAllPartitions() includes temp partitions, but partition limit enforcement/warnings use getPartitionNum() (non-temp). Using allPartitions.size() here can inflate the near-limit gauges (and partitionCount) due to temp partitions and make the metric inconsistent with the actual limit checks. Consider computing near-limit using olapTable.getPartitionNum() / getPartitions() (or otherwise excluding temp partitions) while still iterating getAllPartitions() for size/stat aggregation if needed.
TPC-H: Total hot run time: 26423 ms |
|
run buildall |
TPC-H: Total hot run time: 26444 ms |
TPC-DS: Total hot run time: 168787 ms |
FE UT Coverage ReportIncrement line coverage |
Review: Cloud Mode Not HandledCritical Bug: Cloud mode not handling partition near-limit metrics The PR changes only update The Issue
long autoPartitionNearLimitCount = 0L;
long dynamicPartitionNearLimitCount = 0L;
int tablePartitionNum = allPartitions.size();
partitionCount += tablePartitionNum;
// Check if this table's partition count is near the limit (>80%)
if (olapTable.getPartitionInfo().enableAutomaticPartition()) {
int limit = Config.max_auto_partition_num;
if (tablePartitionNum > limit * 8L / 10) {
autoPartitionNearLimitCount++;
}
}
if (olapTable.dynamicPartitionExists()
&& olapTable.getTableProperty().getDynamicPartitionProperty().getEnable()) {
int limit = Config.max_dynamic_partition_num;
if (tablePartitionNum > limit * 8L / 10) {
dynamicPartitionNearLimitCount++;
}
}
MetricRepo.GAUGE_AUTO_PARTITION_NEAR_LIMIT.setValue(autoPartitionNearLimitCount);
MetricRepo.GAUGE_DYNAMIC_PARTITION_NEAR_LIMIT.setValue(dynamicPartitionNearLimitCount);Impact
This defeats the purpose of the PR for cloud deployments, as users cannot monitor tables approaching partition limits. Fix RequiredApply the same changes to
|
gavinchou
left a comment
gavinchou
left a comment
There was a problem hiding this comment.
Review: Cloud Mode Not Handled
The PR changes only update TabletStatMgr.java, but CloudTabletStatMgr.java is missing the same changes. In cloud mode, Doris uses CloudTabletStatMgr instead of TabletStatMgr (see CloudEnvFactory.java:198-199).
The Issue
CloudTabletStatMgr.updateStatInfo() (lines 156-334) has nearly identical logic to TabletStatMgr.runAfterCatalogReady(), but it is missing:
- Counter variables
- Partition limit check logic
- Gauge metric updates
Impact
- Non-cloud mode: Metrics work correctly (updated by
TabletStatMgr) - Cloud mode: Metrics stay at 0 forever (never updated by
CloudTabletStatMgr)
Fix Required
Apply the same changes to CloudTabletStatMgr.updateStatInfo():
- Add the counter variables
- Add the partition limit check logic inside the table iteration loop
- Add the gauge metric updates before the logging statement
… temp partitions Address review comments: - Add partition near-limit gauge logic to CloudTabletStatMgr.updateStatInfo() so cloud mode also reports these metrics (previously stayed at 0). - Use getPartitionNum() instead of getAllPartitions().size() for the near-limit check to exclude temp partitions, consistent with how partition limits are enforced elsewhere. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
run buildall |
TPC-H: Total hot run time: 26642 ms |
TPC-DS: Total hot run time: 168191 ms |
FE UT Coverage ReportIncrement line coverage |
|
/review |
![github-actions[bot]](https://avatars.gh.lumenfield.work/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
Overview
This PR converts partition near-limit metrics from monotonically increasing LongCounterMetric to GaugeMetricImpl<Long>, and moves computation from inline event-driven sites (FrontendServiceImpl, DynamicPartitionUtil) to the periodic TabletStatMgr/CloudTabletStatMgr scan. The counter-to-gauge conversion is a sound design improvement — gauges correctly reflect the current number of tables near the limit rather than a cumulative event count.
Critical Checkpoint Conclusions
1. Goal & Correctness:
- The auto-partition metric (
auto_partition_near_limit_count) is correct: both the original enforcement site (FrontendServiceImpl) and the new gauge compareolapTable.getPartitionNum()againstConfig.max_auto_partition_num. - Issue found: The dynamic-partition metric has a semantic mismatch (see inline comment). The original
DynamicPartitionUtilcomparedend - start(the configured partition span) againstmax_dynamic_partition_num, but the new code compares total partition count against the same limit. These measure different things.
2. Modification scope: Focused and minimal — 5 files, well-scoped to the metric change.
3. Concurrency: No new concurrency concerns. The gauge values are computed in a single-threaded periodic scan and set atomically via setValue(). The computation reads under existing table read locks.
4. Lifecycle / static init: No issues. Gauge metrics are initialized in MetricRepo.init() before use.
5. Configuration: No new configs added. Existing max_auto_partition_num and max_dynamic_partition_num are reused appropriately.
6. Incompatible changes: Metric names are preserved (auto_partition_near_limit_count, dynamic_partition_near_limit_count). However, semantics changed from cumulative counter to current gauge — any Prometheus alert rules using rate() or increase() on these metrics will silently break. The PR description acknowledges this but there should be a release note.
7. Parallel code paths: Both TabletStatMgr (shared-nothing) and CloudTabletStatMgr (cloud mode) are updated — good.
8. Test coverage: No tests are included. The PR description lists manual test items but no regression or unit tests are added. Given this is a metrics-only change, this is acceptable but not ideal.
9. Observability: The LOG.warn calls are preserved in both FrontendServiceImpl and DynamicPartitionUtil — good.
10. Performance: No concerns. The checks are trivial integer comparisons added to an already-existing iteration loop.
Issues Found
- (Medium) Semantic mismatch in
dynamic_partition_near_limit_count— see inline comments on bothTabletStatMgr.javaandCloudTabletStatMgr.java. - (Minor) Code duplication between
TabletStatMgrandCloudTabletStatMgr— the near-limit computation logic is copy-pasted. Consider extracting to a shared utility method.
| } | ||
| } | ||
| if (olapTable.dynamicPartitionExists() | ||
| && olapTable.getTableProperty().getDynamicPartitionProperty().getEnable()) { |
There was a problem hiding this comment.
Semantic mismatch for dynamic partition near-limit check.
The original metric in DynamicPartitionUtil compared expectCreatePartitionNum (which is end - start, the configured partition span/window) against Config.max_dynamic_partition_num. Per the config's own Javadoc:
Used to limit the maximum number of partitions that can be created when creating a dynamic partition table [...] The number is determined by "start" and "end" in the dynamic partition parameters.
However, the new code here compares nonTempPartitionNum (total current partition count on the table) against max_dynamic_partition_num. These are semantically different:
- A table with
start=-5, end=3(span=8) but 17,000 manually-added partitions would trigger this gauge even though the dynamic partition config is well within the limit. - Conversely, a table with
start=-15000, end=5000(span=20,000) but only 100 currently-existing partitions would NOT trigger the gauge, even though the dynamic partition span already hits the hard limit.
Consider comparing the configured span (end - start) instead:
DynamicPartitionProperty dpProp = olapTable.getTableProperty().getDynamicPartitionProperty();
long span = (long) dpProp.getEnd() - dpProp.getStart();
int limit = Config.max_dynamic_partition_num;
if (span > limit * 8L / 10) {
dynamicPartitionNearLimitCount++;
}This would make the gauge semantically consistent with the enforcement in DynamicPartitionUtil.
Note: The auto-partition check above is correct — FrontendServiceImpl.createPartition() does compare total partition count against max_auto_partition_num.
| } | ||
| } | ||
| if (olapTable.dynamicPartitionExists() | ||
| && olapTable.getTableProperty().getDynamicPartitionProperty().getEnable()) { |
There was a problem hiding this comment.
Same semantic mismatch as in TabletStatMgr.java — the dynamic partition near-limit check should compare the configured span (end - start) against max_dynamic_partition_num, not the total partition count. See the detailed comment on the TabletStatMgr.java counterpart.
github-actions
bot
added
the
approved
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
…uges (#61845) ## Summary - Changed `auto_partition_near_limit_count` and `dynamic_partition_near_limit_count` from `LongCounterMetric` (monotonically increasing) to `GaugeMetricImpl<Long>` so they correctly decrease when the near-limit condition resolves - Moved metric computation from inline event-driven increments (in `FrontendServiceImpl` and `DynamicPartitionUtil`) to `TabletStatMgr`'s periodic all-table scan, which already iterates all tables and partitions under read locks - Metric names are preserved for monitoring compatibility; semantics changed from "cumulative event count" to "current number of tables near the limit" ## Test plan - [ ] Verify `auto_partition_near_limit_count` increases when an auto-partition table exceeds 80% of `max_auto_partition_num` - [ ] Verify the gauge decreases back to 0 after dropping partitions below the 80% threshold (within one `tablet_stat_update_interval_second` cycle) - [ ] Verify `dynamic_partition_near_limit_count` behaves the same for dynamic partition tables - [ ] Verify existing Prometheus/Grafana dashboards continue to scrape the metric names without changes 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
auto_partition_near_limit_countanddynamic_partition_near_limit_countfromLongCounterMetric(monotonically increasing) toGaugeMetricImpl<Long>so they correctly decrease when the near-limit condition resolvesFrontendServiceImplandDynamicPartitionUtil) toTabletStatMgr's periodic all-table scan, which already iterates all tables and partitions under read locksTest plan
auto_partition_near_limit_countincreases when an auto-partition table exceeds 80% ofmax_auto_partition_numtablet_stat_update_interval_secondcycle)dynamic_partition_near_limit_countbehaves the same for dynamic partition tables🤖 Generated with Claude Code