[feature](fe) Add external metadata cache memory governance - #67726
Open
CalvinKirs wants to merge 1 commit into
Open
[feature](fe) Add external metadata cache memory governance#67726CalvinKirs wants to merge 1 commit into
CalvinKirs wants to merge 1 commit into
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
CalvinKirs
force-pushed
the
master-memory-cache
branch
from
September 9, 2026 09:44
f684d9f to
3affaf5
Compare
Member
Author
|
run buildall |
Member
Author
|
/review |
CalvinKirs
force-pushed
the
master-memory-cache
branch
from
September 9, 2026 10:10
3affaf5 to
5a75b57
Compare
### What problem does this PR solve? Issue Number: None Related PR: apache#66717 Problem Summary: External catalog metadata caches were bounded primarily by entry count, so large Hive, Iceberg, and Paimon metadata objects could consume unpredictable FE heap and cause OOM. Add bounded retained-size estimation, hierarchical global/catalog/entry budgets, coordinated local reclamation, rejection observability, and catalog cache statistics. Keep estimation off the cache-hit path, preserve unbounded behavior when no weight limit is configured, and prevent managed cache owners from escaping connector lifecycle or registry concurrency. Include the Hive gateway's Hudi sibling HMS caches in shared aggregate accounting and statistics while preserving their existing entry-local configuration namespace. ### Release note Add configurable memory limits and usage statistics for external metadata caches. ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.connector.cache.MetaCacheWeightGovernanceTest,org.apache.doris.connector.hive.HiveConnectorSiblingTest,org.apache.doris.connector.paimon.PaimonCatalogFactoryTest - EXTRA_FE_MODULES=hudi=fe-connector/fe-connector-hudi ./run-fe-ut.sh --run HudiConnectorHmsCacheTest,HudiResourceLifecycleTest,MetaCacheWeightGovernanceTest,HiveConnectorSiblingTest - Paimon partition weighting: 571 connector unit tests passed; one opt-in live-connectivity test skipped. FE/plugin packaging and Checkstyle passed. Three fresh JVM microbenchmark runs and independent local JOL calibration completed. - Behavior changed: Yes (external metadata caches can be governed by retained-size limits and expose usage/rejection statistics) - Does this need documentation: Yes (maintained in the Doris website documentation change)
CalvinKirs
force-pushed
the
master-memory-cache
branch
from
September 9, 2026 10:41
5a75b57 to
9010fa1
Compare
Member
Author
|
run buildall |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Requesting changes: six distinct issues remain on exact head 9010fa1.
Blocking findings:
- P1: exact-key invalidation can leave a resident value unaccounted.
- P1: Hive and Iceberg sampled variable payloads are falsely marked complete.
- P1: an unknown future
max-weightkey can block every unrelated ALTER after downgrade. - P2: a disabled Iceberg table cache serializes and estimates full metadata on every live load under an enclosing limit.
- P2: changing the effective catalog maximum can race lazy reinitialization against the old core owner.
- P1: new FE statistics slots fail on an old BE before the RPC fallback can run.
Checkpoint conclusions:
- Goal and proof: the managed memory-governance design is cohesive and has substantial coverage, but the quota, generation, configuration-compatibility, and rolling-upgrade invariants above are not met; the decisive regression tests are missing.
- Scope: the authoritative 79-file patch is focused on governance and connector integration.
- Concurrency and locks: peer reclaim, ordinary replacement, and close lock ordering look sound, with no additional deadlock found; findings 1 and 5 are the remaining races.
- Lifecycle and static state: parent-first governance identity, initialization, and ordinary owner cleanup are sound; dynamic rebuilding still has finding 5.
- Configuration and persistence: global restart-only and catalog dynamic routing were traced; findings 3 and 5 break downgrade and dynamic-update guarantees. There are no storage-format or transactional-write changes.
- Compatibility and nullability: new-BE/old-FE RPC fallback and 24/28-cell materialization are safe, and appended nullable defaults otherwise look sound, but the new-FE/old-BE path in finding 6 remains broken.
- Parallel and special paths: core, Remote Doris, Hive/HMS/Hudi, Iceberg, and Paimon were traced. Finding 2 affects Hive and both Iceberg estimators. Explicit Paimon SDK caching is an intentional unmanaged opt-in and is not raised.
- Tests and results: coverage is broad, but deterministic same-node, skewed-tail, future-key ALTER, disabled-plus-bounded, real ALTER/reinit, and new-FE/old-BE slot tests are missing. This was a review-only run, so I did not build or execute tests; the PR's reported old-BE validation reproduces finding 6.
- Observability: the new fields and warnings are appropriate, but findings 1 and 2 can underreport usage and finding 6 makes the fields unusable during upgrade.
- Performance: estimation is generally confined to load/publication paths, but finding 4 adds avoidable full serialization to a deliberately disabled path.
Overall: request changes. No additional user focus was supplied. The review converged after three bounded rounds, with every final agent returning NO_NEW_VALUABLE_FINDINGS.
| && current.keyState == invalidated.keyState) { | ||
| data.asMap().remove(key, current); | ||
| } | ||
| if (registered != null && invalidated.node.registration.compareAndSet(registered, null)) { |
Contributor
There was a problem hiding this comment.
[P1] Keep invalidation cleanup tied to the invalidated generation. A put can reuse this KeyNode after the state swap and install a new registration before cleanup reaches this CAS; cleanup then leaves the new value in Caffeine but clears/releases its reservation. Please compare against a registration tied to the invalidated KeyState and add a deterministic same-node test without the intervening getIfPresent.
| bytes = add(bytes, JvmSizeUtils.stringSize(key.getTable())); | ||
| bytes = add(bytes, ARRAY_LIST_SHALLOW_BYTES); | ||
| bytes = add(bytes, JvmSizeUtils.objectArraySize(partitions.size())); | ||
| bytes = add(bytes, JvmSizeUtils.sampledListPayload( |
Contributor
There was a problem hiding this comment.
[P1] Do not mark a sampled variable-length payload complete. A large partition at an unsampled index is retained but absent from both this sample and the reflective fallback, so the entry can be admitted below its real weight. The same bug is present in both Iceberg derived-view estimators; use an exhaustive construction-time payload pass (as Paimon does) or return incomplete, and add skewed-tail tests.
| } | ||
| String entry = key.substring(prefix.length(), key.length() - KEY_MAX_WEIGHT.length()); | ||
| if (!entries.contains(entry)) { | ||
| throw new IllegalArgumentException("Unknown metadata cache weight property: " + key); |
Contributor
There was a problem hiding this comment.
[P1] Preserve ALTER after downgrade. This binary can replay a catalog containing a future meta.cache.<engine>.<entry>.max-weight key, but the default update validator merges that key into every unrelated ALTER and this branch rejects it; ALTER CATALOG cannot remove properties. Distinguish unknown keys introduced by this statement from unknown keys already persisted so CREATE/new typos may still fail, and add a future-key plus unrelated-ALTER test.
| } | ||
| }; | ||
| TableOwner loaded = new TableOwner(table, cleanup, true); | ||
| TableOwner loaded = new TableOwner(table, cleanup, true, entry.isWeightBounded()); |
Contributor
There was a problem hiding this comment.
[P2] Skip weight preparation when this cache is disabled. With ttl-second=0 or enable=false plus a global/catalog limit, isWeightBounded is still true, so every live table load serializes and estimates the full TableMetadata even though the disabled path immediately discards it. Gate this work on entry.isEnabled() as well and cover the combined configuration.
| ExternalMetaCacheMgr cacheMgr = Env.getCurrentEnv().getExtMetaCacheMgr(); | ||
| if (updatedProps.get(SCHEMA_CACHE_TTL_SECOND) != null) { | ||
| if (updatedProps.get(SCHEMA_CACHE_TTL_SECOND) != null | ||
| || updatedProps.containsKey(MetaCacheBudgetManager.CATALOG_MAX_WEIGHT_PROPERTY)) { |
Contributor
There was a problem hiding this comment.
[P2] Retire old catalog budgets before allowing reinitialization. When this ALTER changes the effective maximum, resetToUninitialized releases the catalog monitor before removeCatalog runs, so a concurrent makeSureInitialized can build the new connector while the default-engine owner still holds the old catalog bucket; createEntryBudget then throws a conflicting-limit error. Fence reinitialization through removal and add a real old/new-budget interleaving test.
| .column("LAST_LOAD_SUCCESS_TIME", ScalarType.createStringType()) | ||
| .column("LAST_LOAD_FAILURE_TIME", ScalarType.createStringType()) | ||
| .column("LAST_ERROR", ScalarType.createStringType()) | ||
| .column("MAX_WEIGHT", ScalarType.createType(PrimitiveType.BIGINT)) |
Contributor
There was a problem hiding this comment.
[P1] Keep the appended slots usable during a rolling upgrade. A new FE can put MAX_WEIGHT (and the other new columns) in the destination tuple sent to an old BE, but the old SchemaScanOperatorX rejects that unknown slot during prepare, before this PR's FE-RPC 28-to-24 fallback can run. Gate planning/routing by BE compatibility and add a mixed-version test that selects and filters on MAX_WEIGHT.
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 16765 ms |
Contributor
TPC-DS: Total hot run time: 83285 ms |
Contributor
ClickBench: Total hot run time: 14.71 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Related PR: #66717 (merged into
branch-4.1)Problem Summary:
Port external metadata cache memory governance to
master, adapting it to the connector SPI and shared cache framework rather than cherry-picking the legacy 4.1 cache implementation unchanged.Master adaptation: rebased onto
1335022902a. Preserved upstream DLF support when resolving the Iceberg/Paimon connector conflicts and adapted three assertions in the newly added FE cache test to master's JUnit 5 migration. No test expectations or additional production behavior were changed by conflict resolution.Entry-count limits alone cannot control FE heap when external metadata values vary substantially in size. This change adds estimated retained-memory budgets for managed caches while keeping existing count-based behavior when no applicable memory limit is configured.
Scope and design
MAX_WEIGHT,ESTIMATED_WEIGHT,WEIGHT_REJECT_COUNT, andLAST_WEIGHT_REJECT_REASONthroughinformation_schema.catalog_meta_cache_statistics, with rate-limited rejection warnings.No optimizer rules, literal representations, or partition-item implementations are changed.
Configuration and behavior
Catalog/entry property changes rebuild the affected cache owners without restarting FE. An oversized value, or one that cannot obtain budget after reclamation, is returned to the current request without being retained in the cache. Normal admission rejection does not fail the query. These are limits on estimated retained cache memory, not pre-load heap reservations or a guarantee against OOM while constructing a large value; active query objects and shared infrastructure are outside this budget.
Paimon microbenchmark (master adaptation)
Added a standalone Paimon benchmark (171 lines) plus two fixture-validation tests (67 lines), with no new dependency or production-code change. This is a microbenchmark harness, not JMH. A fresh focused run passed 33 tests (0 failures/errors/skips); module Checkstyle/architecture validation passed.
Three fresh Java 17 JVMs,
-Xms1g -Xmx4g -XX:ActiveProcessorCount=4, 5 warmup and 15 measurement windows per operation. The benchmark calls the production partition collector with recording catalog fakes, using 10/100 schema fields, 1,000/10,000 partitions, and uniform/sampled-tail/unsampled-tail distributions. It rotates operation order and reports actual operation counts and min/median/max window timings. No remote RPC, object-store latency, data scan, cache-hit throughput, or full admission-lock cost is included.Representative uniform cases (median of the three JVM medians):
The small +1.3% to +4.1% differences overlap run variation and are not precise overhead guarantees. All-case publication-only medians are 3.76–8.51 us; prepared-weight callback timing is about 14 ns including loop/volatile-sink overhead, not complete cache admission. Each fork measures 120–3,840 collection operations and 30,720–245,760 publication operations per case. This is not a claim of end-to-end query speedup.
Release note
Add configurable estimated-memory limits, dynamic catalog/entry quota changes, and memory usage/rejection statistics for managed external metadata caches.
Check List (For Author)
Test
f684d9fe479): existingexternal_table_p0/test_catalog_ddlpassed on a running cluster (1 suite, 0 failures/fatal errors/skips). Not rerun against a deployed post-rebase FE.f684d9fe479): local integration suite executed withrun-regression-test.sh, followed by a separate comparison run against automatically generated and inspected output (1 suite, 0 failures/fatal errors/skips). Actual Iceberg REST/MinIO data: 256 partitions / 8,192 rows. Verified catalog-only quotas with global quota disabled,64MB -> 1B -> 64MB, entry-local rejection/recovery, rejection observability, and identical query results with/without cache admission (COUNT(*)=8192,SUM(id)=33550336).build.sh --feinto an isolated output directory; Checkstyle andgit diff --checkpassed. A temporary FE-only guard avoided an unrelated missing native-thirdparty rebuild; that environment adjustment was restored and is not included in this PR. The running pre-rebase test cluster was not replaced.no match column ... MAX_WEIGHT). Memory statistics were checked through the running FE's Thrift endpoint. Matching-master BE compilation/system-table end-to-end validation, full external-engine integration, and follower replay remain unverified by this local run. No new end-to-end performance claims are made.Behavior changed:
Check List (For Reviewer who merge this PR)