Repository navigation
[improvement](fe) Add the branch-4.x weight budget columns to catalog_meta_cache_statistics - #69557
Conversation
…_meta_cache_statistics ### What problem does this PR solve? Issue Number: None Related PR: apache#66717, apache#67726 Problem Summary: branch-4.x (apache#66717) shows ten memory-weight columns in information_schema.catalog_meta_cache_statistics. Master (apache#67726) has only MAX_WEIGHT, ESTIMATED_WEIGHT, WEIGHT_REJECT_COUNT and LAST_WEIGHT_REJECT_REASON, so monitoring queries written for 4.x, and the weight scenarios of 4.x's own regression suites, fail on master with unknown columns: WEIGHT_BOUNDED, EVICTION_WEIGHT, CATALOG_MAX_WEIGHT, CATALOG_ESTIMATED_WEIGHT, GLOBAL_MAX_WEIGHT and GLOBAL_ESTIMATED_WEIGHT. For an entry without a weight bound master also reports ESTIMATED_WEIGHT and WEIGHT_REJECT_COUNT as 0, where 4.x reports -1. The six columns are appended after LAST_WEIGHT_REJECT_REASON with the 4.x names, types and meanings: whether the entry is weight bounded; the bytes of the values removed by eviction, by Caffeine or to make room in the budget; the catalog limit the entry is admitted under (the smaller of the catalog and FE-global limits) and the bytes charged to its catalog; the FE-global limit and the bytes charged to it. A limit that does not exist is -1, and an entry without a weight bound reports -1 in every weight column, as on 4.x. The BE scanner asks an older FE that rejects the new columns for the leading 28, then 24 columns. SELECT * positions still differ from 4.x, whose base columns lack EVICTION_RATE; queries by column name now work on both. ### Release note information_schema.catalog_meta_cache_statistics adds the columns WEIGHT_BOUNDED, EVICTION_WEIGHT, CATALOG_MAX_WEIGHT, CATALOG_ESTIMATED_WEIGHT, GLOBAL_MAX_WEIGHT and GLOBAL_ESTIMATED_WEIGHT. A cache entry without a weight bound now reports -1 instead of 0 in ESTIMATED_WEIGHT and WEIGHT_REJECT_COUNT. ### Check List (For Author) - Test: Regression test / Unit Test - MetaCacheWeightGovernanceTest, FeMetaCacheEntryTest, HudiConnectorHmsCacheTest - test_paimon_table_meta_cache and test_remote_doris_meta_cache_weight, run against a local cluster; test_iceberg_table_meta_cache needs the hive3 environment and was not run locally - Manual: a new BE against an FE without the six columns falls back to the leading 28 - Behavior changed: Yes. A cache entry without a weight bound reports -1 instead of 0 in ESTIMATED_WEIGHT and WEIGHT_REJECT_COUNT. Upgrade BE before FE: an older BE cannot fill the six new columns. - Does this need documentation: Yes. The catalog_meta_cache_statistics page should list the six columns (doc PR to follow). Co-authored-by: Calvin Kirs <[email protected]> Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
There was a problem hiding this comment.
Reviewed all 14 changed files at eef214d. One P2 issue is reported inline: concurrent invalidation can release a reservation before deferred eviction accounting reads its bytes, permanently understating EVICTION_WEIGHT. No existing inline findings were present, and no additional user focus was supplied.
The inspected flow is:
CatalogMetaCache / EntryBudget
-> ScopedMetaCache admission, replacement and deferred removal
-> CacheMetrics -> MetaCacheEntryStats -> MetadataGenerator TRow/TCell
-> BE catalog-meta-cache scanner -> information_schema SQL columns
Applicable code-review skill checkpoints:
| Checkpoint | Conclusion |
|---|---|
| Goal and evidence | Six weight columns and unbounded numeric -1 sentinels reach the SQL view. Added tests cover nominal eviction, hierarchy and sentinels; the inline finding identifies an uncovered eviction/invalidation race. |
| Scope and reuse | Focused changes reuse shared budget accounting and the existing snapshot/serializer/scanner pipeline. |
| Concurrency and locks | Request workers, refresh workers, direct Caffeine maintenance and peer reclamation can interleave. Manager arithmetic is locked; reclaim callbacks run outside that lock. No new lock inversion or heavy I/O under locks found. The lifetime of the sampled eviction weight is incorrect in the reported race. |
| Lifecycle and initialization | Reservation replacement/rollback and exactly-once release were traced. New metrics add scalar state with no retaining cycle or cross-TU initialization dependency; eviction event weight must survive reservation release. |
| Configuration | No new configuration key. Existing entry, catalog and global bounds, disabled caches and unbounded entries were inspected. |
| Compatibility | FE declarations, encoded cells and BE descriptors agree on 34 ordered columns. New BE retries old FE with 28 then 24 columns and fills missing tails with NULL. BE must be upgraded before querying the new columns through an upgraded FE; this is the documented rollout restriction. No persisted format or connector SPI definition changes. |
| Thrift/protobuf IDs | No definition files changed; release-branch ID comparison is not applicable. |
| Parallel paths | Load, put, CAS, refresh and bulk publication share the reviewed removal machinery. Capacity, expiry and budget eviction reach the common counter; explicit invalidation and replacement are excluded. Builtin and connector statistics share the FE serializer. |
| Conditions and errors | Bounded published values have a nonnull reservation by construction. That guarantee does not preserve its mutable byte count until deferred removal. Retry widths are documented; transport and terminal FE errors still propagate. |
| Test coverage | Unit coverage includes nominal capacity/budget eviction, invalidation exclusion, hierarchy and unbounded values; regression changes cover Paimon, Iceberg and remote Doris. Expiry/invalidation ordering and direct mixed-version scanner tests are missing. |
| Changed test results | Both added .out files have deterministic order_qt projections and expected entry sets/types under the default unlimited-global configuration. Their generation history and execution were not verified. |
| Observability | Added fields expose boundedness and budget usage; existing rejection reasons and FE-host failure logs remain. The reported issue permanently loses eviction-byte events. Hierarchy getters sample different instants, consistent with the surrounding approximate statistics; no functional consumer relies on cross-field atomicity. |
| Transactions and persistence | No transaction, visible-version, delete-bitmap, EditLog or persisted metadata behavior changes. |
| Data writes and crashes | Implementation changes concern transient cache statistics; no data-write atomicity or newly introduced crash leak/deadlock defect established. |
| FE/BE field passing | All six fields are present in FE schema, shared snapshot conversion, TCell encoding and BE schema/decoding. No alternate constant-folding or point-query variable path applies. |
| Performance and memory | Added work is constant-time primitive accounting, short budget-lock reads and bounded fallback retries. Saturating addition prevents cumulative overflow. No significant untracked allocation or new retained object graph. |
| Other concerns and final recheck | Rechecked null/disabled values, replacement generations, expiry, exact/scope invalidation, close, old-FE status conversion, boolean decoding and nullable tails. No additional substantiated finding or unresolved code candidate remains. |
Coverage limits: this was a static-only review. No builds, unit/regression tests, cluster runs or rolling-upgrade experiments were executed; runtime behavior and .out generation provenance remain unverified. The review ledger records each candidate and risk disposition. The separate hierarchy-snapshot concern was dismissed with the consumer and lock evidence above.
Review converged after 2 round(s).
| if (weightBounded) { | ||
| // Every value of a bounded cache holds a reservation; it is released below. | ||
| evictionWeight.accumulateAndGet( | ||
| versioned.registration.reservation.getBytes(), JvmSizeUtils::saturatedAdd); |
There was a problem hiding this comment.
[P2] Preserve the admitted byte count through deferred eviction accounting
An eviction can permanently add zero bytes here. After thread A removes a weighted value for expiry, capacity or budget reclamation, its deferred callback can pause before this read. Thread B can then call invalidateKey(key): the old registration is still attached, the value is already absent from data, and the registration CAS/release at lines 490-491 calls MetaCacheBudgetManager.release(), which sets reservation.bytes to 0. When A resumes, evictionCount increments but EVICTION_WEIGHT loses that value's entire weight. The deferred drain holds no publication gate or key-node monitor that prevents this schedule. Preserve the admitted generation's immutable weight for the removal event instead of reading the live reservation after another path can release it, and cover eviction racing with invalidation.
There was a problem hiding this comment.
Confirmed, and fixed in 31dba971849. Releasing an AdmissionReservation zeroes its bytes, and an exact invalidateKey releases the generation its key node still points to, by a CAS outside the key node's monitor, so a release landing between Caffeine's removal and the deferred drain made the eviction count 0 bytes. Quota accounting was not affected: the reservation is still released exactly once. Each Registration now records the bytes its reservation was admitted with when the generation is created (both the fresh admission and the replacement create it right after admission), and completeRemoval adds those; reading the reservation under the key node's monitor would not have helped, since the invalidation's release does not take it. Budget reclamation in evictLocalColdestWithDeferredRemovals reads the same field for the bytes it removed, which equals the live value of a reservation that is not released and needs no budget manager lock. MetaCacheWeightGovernanceTest#evictionWeightKeepsTheAdmittedBytesWhenInvalidationReleasesFirst keeps the removal deferral open inside forEach, queues a capacity eviction and invalidates both keys before the drain: EVICTION_WEIGHT is now 768, where it was 0, and the budget is fully released.
TPC-H: Total hot run time: 28452 ms |
TPC-DS: Total hot run time: 150648 ms |
ClickBench: Total hot run time: 24.2 s |
FE UT Coverage ReportIncrement line coverage |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
FE Regression Coverage ReportIncrement line coverage |
### What problem does this PR solve? Issue Number: None Related PR: apache#69557 Problem Summary: EVICTION_WEIGHT of catalog_meta_cache_statistics adds the bytes of every value Caffeine evicts or budget reclamation removes. The removal is handled by a deferred drain on the evicting thread, which read the live reservation of the evicted generation. An exact invalidation of the same key can release that generation's reservation between the eviction and the drain (the key node still points to it, and the invalidation releases it outside the key node's monitor), and releasing a reservation zeroes its bytes, so the eviction was counted with 0 bytes and EVICTION_WEIGHT stayed short for good. Quota accounting was not affected: the reservation is still released exactly once. Each generation's registration now records the bytes its reservation was admitted with when the generation is created, and eviction accounting adds those. Budget reclamation reads the same field for the bytes it removed, which equals the live value of a reservation that is not released and needs no budget manager lock. ### Release note None ### Check List (For Author) - Test: Unit Test - MetaCacheWeightGovernanceTest#evictionWeightKeepsTheAdmittedBytesWhenInvalidationReleasesFirst: inside forEach, which defers removals, a capacity eviction is queued and both keys are invalidated before the drain; EVICTION_WEIGHT is the admitted bytes (it was 0) and the budget is fully released - fe-connector-cache module tests - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
|
Addressed the review in
One new test in |
|
run buildall |
|
/review |
There was a problem hiding this comment.
Reviewed head 31dba971849fb187515b97da98ba42518921df37, covering all 14 changed files and their relevant callers, lifecycle and parallel paths. No new actionable findings identified.
Existing P2 comment 4365835392 is addressed: both fresh and replacement generations capture immutable admitted bytes before publication, and deferred eviction accounting reads that value after any reservation release. The added release-before-drain test exercises the reported ordering. No duplicate comment is posted, and no existing P0/P1 finding remains in the supplied/live thread context. No additional user focus was supplied.
The reviewed flow is CatalogMetaCache / EntryBudget -> ScopedMetaCache admission and removal -> CacheMetrics -> MetaCacheEntryStats -> ExternalMetaCacheMgr -> MetadataGenerator -> schema-table RPC -> BE scanner -> SQL projection.
Critical checkpoint conclusions:
- Goal and supporting tests: the six added columns and unbounded numeric
-1values are carried through the complete flow. Added unit and regression cases support the intended bounded/unbounded, hierarchy and eviction behavior. - Scope and reuse: changes are focused on cache observability, reuse the existing stats/RPC/scanner paths and add no unrelated production behavior.
- Concurrency: query/load/refresh/invalidation threads and the peer-reclaim worker were traced. Manager arithmetic uses its existing lock; release uses registration CAS; eviction-byte addition is atomic and saturating. Hierarchy gauges remain independently sampled observations.
- Lock work and order: new reads and counter updates are constant time. Reclamation runs outside the manager lock and deferred removals drain outside publication/Caffeine maintenance locks; no new reverse lock edge was found.
- Lifecycle and initialization: immutable admission bytes retain no cached-value graph. Fresh admission, replacement commit/rollback, invalidation and close preserve ownership/release paths. No new static initialization dependency or reference cycle.
- Configuration: no new setting or dynamic-update behavior; metrics reflect the existing entry/catalog/global budget lifecycle.
- Compatibility: FE and BE agree on all 34 names, positions and types. New BE retries older FE schemas with 28 then 24 columns, padding unavailable trailing cells with NULL; old BE requests retain their existing filtered columns. Materializing added columns requires the documented BE-before-FE upgrade order.
- Thrift/protobuf definitions: no field IDs, enum values or generated definitions changed; existing RPC cells carry the values.
- Parallel paths: FE-owned and plugin-owned stats, fresh/replacement/refresh/bulk publication, and expiry/capacity/peer/invalidation/close removal paths were checked. Shared constructors and removal accounting cover the affected paths.
- Special conditions: bounded/unbounded branches follow the constructor-established estimator/budget pairing. The deferred-release and older-schema conditions have concrete explanatory comments.
- Test coverage: reviewed hierarchy and sentinel cases, capacity and budget eviction, explicit invalidation exclusion, rejected mutation, and the deterministic release-before-drain case, alongside relevant existing concurrency/rollback coverage. Rolling-upgrade fallback was checked statically.
- Test results: both new result files match selected columns, registered entry names, 128 MiB limits and expected sentinels; ordered query checks provide deterministic output. Result-generation provenance was not independently verified.
- Observability: bounded status, eviction bytes and enclosing-budget usage provide the intended diagnostics; no new heavy logging or external I/O.
- Transactions and persistence: no EditLog, transaction visibility, delete bitmap, storage format or failover protocol change.
- Writes and crash atomicity: production changes expose transient cache state; existing publication, accounting rollback and exactly-once release protections remain in place. No new durable write path.
- FE/BE propagation: schema registration, stats mapping, TCell construction, RPC filtering, scanner decoding and projection agree; no alternative send path applies.
- Performance and memory: constant-size metric additions and one primitive admitted-weight field per generation; no new graph traversal or unbounded retry. Compatibility fallback adds at most two RPC attempts.
- Further recheck: revisited delayed old-generation cleanup, pending replacement/removal overlap, close-before-publication, absent enclosing limits, empty old-FE batches and added-column projection. No further substantiated issue or unresolved static candidate remains.
Coverage limits: review was static only. No builds, tests, runtime concurrency experiment or mixed-version cluster validation was executed in this invocation. These limits apply to the verification method; all assigned changed-file and risk scopes were reviewed.
Review converged in 1 round: both complete-review agents and the separate risk reviewer returned NO_NEW_VALUABLE_FINDINGS, and the final main-agent sweep found no uncovered changed path or unresolved candidate.
Review converged after 1 round(s).
TPC-H: Total hot run time: 28557 ms |
TPC-DS: Total hot run time: 150763 ms |
ClickBench: Total hot run time: 24.3 s |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
FE Regression Coverage ReportIncrement line coverage |
What problem does this PR solve?
Issue Number: None
Related PR: #66717, #67726
Problem Summary:
In short. This brings the weight columns of
information_schema.catalog_meta_cache_statisticsfrom branch-4.x #66717 to master. Master shows four of the ten: this PR addsWEIGHT_BOUNDED,EVICTION_WEIGHT,CATALOG_MAX_WEIGHT,CATALOG_ESTIMATED_WEIGHT,GLOBAL_MAX_WEIGHTandGLOBAL_ESTIMATED_WEIGHTwith the 4.x names, types and meanings, and makes a cache entry without a weight bound report-1in every weight column, as 4.x does, instead of0inESTIMATED_WEIGHTandWEIGHT_REJECT_COUNT.Background. External catalogs can bound the memory of their metadata caches: an FE-global limit (
external_meta_cache_max_weight), a catalog limit (meta.cache.max-weight) and per-entry limits.catalog_meta_cache_statisticsis how users watch those caches. On branch-4.x (#66717) the view tells, per entry, whether it is weight bounded, what it holds and has evicted, and how full its catalog and the FE-global budget are. Master (#67726) re-implemented the governance on the connector cache framework and exposed onlyMAX_WEIGHT,ESTIMATED_WEIGHT,WEIGHT_REJECT_COUNTandLAST_WEIGHT_REJECT_REASON.The problem, and what it cost. Monitoring queries written for 4.x fail on master with an unknown column as soon as they name one of the other six, and so do the weight scenarios of 4.x's
test_paimon_table_meta_cacheandtest_iceberg_table_meta_cache. There is also no way on master to see how much of a catalog or of the FE-global budget is used, or how many bytes were evicted to make room. And an entry without a weight bound reportsESTIMATED_WEIGHT = 0andWEIGHT_REJECT_COUNT = 0, which reads like an empty but bounded cache.How this PR fixes it.
MetaCacheBudgetManager.EntryBudgetexposes the catalog limit the entry is admitted under (the smaller of the catalog and FE-global limits, as the admission check uses it), the bytes charged to its catalog, the FE-global limit and the bytes charged to it. A limit that does not exist is-1.ScopedMetaCacheadds the bytes each value was admitted with to an eviction-weight counter when it removes the value by eviction, by Caffeine (capacity or expiry) or to make room in the budget. The bytes are recorded when the value is admitted, because an exact invalidation can release the reservation of an evicted value before the eviction is accounted. Invalidation and replacement do not count.ScopedMetaCache.CacheMetrics,MetaCacheEntryStats,SchemaTableandMetadataGeneratorcarry the six values. The columns are appended afterLAST_WEIGHT_REJECT_REASON. For an entry without a weight bound, every weight column is-1.SELECT *positions still differ from 4.x, because master's base columns also haveEVICTION_RATE; queries that name their columns now work on both.Results.
SELECT weight_bounded, catalog_max_weight ... FROM catalog_meta_cache_statisticsmeta.cache.max-weight = 128MBMAX_WEIGHT = 134217728onlyCATALOG_MAX_WEIGHT = 134217728, the catalog's and the FE-global charges, the evicted bytesESTIMATED_WEIGHT = 0,WEIGHT_REJECT_COUNT = 0-1in every weight columnTests:
MetaCacheWeightGovernanceTest: an entry without a weight bound reports-1everywhere; the catalog and FE-global limits and charges of bounded entries, with and without an FE-global limit; a budget reclaim and a capacity eviction add the evicted reservation to the eviction weight, an invalidation does not. An eviction keeps the bytes it was admitted with when the key is invalidated before the deferred eviction is accounted. Restoring master's0for unbounded entries, or dropping the eviction accounting, turns these tests red.FeMetaCacheEntryTestchecks the fe-core view of an unbounded and of a bounded entry;HudiConnectorHmsCacheTestnow expects-1for its unbounded entry.test_paimon_table_meta_cachegets the weight scenario of 4.x on master's entry names (a catalog withmeta.cache.max-weight = 128MBnext to an unbounded one);test_iceberg_table_meta_cacheputs its first catalog under a catalog limit and checks the budget hierarchy of its bounded entries, as 4.x does; the newtest_remote_doris_meta_cache_weightcovers thedorisengine without any external service.Original author: @CalvinKirs
Release note
information_schema.catalog_meta_cache_statisticsadds the columnsWEIGHT_BOUNDED,EVICTION_WEIGHT,CATALOG_MAX_WEIGHT,CATALOG_ESTIMATED_WEIGHT,GLOBAL_MAX_WEIGHTandGLOBAL_ESTIMATED_WEIGHT. A metadata cache entry without a weight bound now reports-1instead of0inESTIMATED_WEIGHTandWEIGHT_REJECT_COUNT.Check List (For Author)
Test
With an FE built before this PR and a BE built from it,
SELECT ... FROM information_schema.catalog_meta_cache_statisticsreturns the rows; the FE log shows the rejected 34-column request.test_paimon_table_meta_cacheandtest_remote_doris_meta_cache_weightwere run against a local cluster and generated their.outfiles.test_iceberg_table_meta_cacheneeds the hive3 environment and was not run locally; its new check uses assertions only.Behavior changed:
A metadata cache entry without a weight bound reports
-1instead of0inESTIMATED_WEIGHTandWEIGHT_REJECT_COUNT. Upgrade BE before FE: an older BE cannot fill the six new columns.Does this need documentation?
The page of
catalog_meta_cache_statisticsshould list the six columns (doc PR to follow).Check List (For Reviewer who merge this PR)