Repository navigation
[improvement](fe) Validate submitted meta.cache keys of the catalog's own engines - #69556
Conversation
… own engines ### What problem does this PR solve? Issue Number: None Related PR: apache#66717, apache#67726 Problem Summary: branch-4.x (apache#66717) validates every meta.cache.* catalog property against a whitelist at CREATE / ALTER CATALOG. Master (apache#67726) rejects only an unknown .max-weight entry that the statement submits and checks the values of a few keys, so a misspelled option (meta.cache.hive.file.max-weigth), enable=1 (read by Boolean.parseBoolean as false, which silently turns the cache off) and ttl-second=-2 are all accepted. Master also accepts a catalog quota above the FE global quota (external_meta_cache_max_weight) and only clamps it at run time, so SHOW CREATE CATALOG promises more memory than the caches get. Every key that a CREATE or ALTER CATALOG submits under meta.cache.<engine>. for an engine the catalog owns must now name a known option (enable, ttl-second, capacity, max-weight) with a value of its type: enable is true or false, ttl-second >= -1, capacity >= 0 and max-weight a positive size. The check sits in CacheSpec.checkWeightProperties, which the hive (with its iceberg sibling), iceberg, paimon, max_compute and adbc connectors already call with the submitted properties, and in a new ExternalCatalog hook that CatalogMgr calls on CREATE and ALTER for the namespace fe-core owns (default, or doris for a remote Doris catalog); replay never runs it. Unknown entries stay accepted, since a newer connector may read an entry this version does not know, and keys an earlier statement stored are not validated again, so a stored key never blocks an unrelated ALTER. A submitted catalog quota, and a submitted entry quota of a catalog without one, may not exceed the FE global quota of the FE running the statement. ### Release note CREATE / ALTER CATALOG now rejects a meta.cache.<engine>.<entry>.<option> property whose option is not enable, ttl-second, capacity or max-weight or whose value has the wrong type, and a meta.cache.max-weight (or an entry max-weight of a catalog without one) above external_meta_cache_max_weight. ### Check List (For Author) - Test: Unit Test, Regression test - CacheSpecTest; HiveCatalogPropertiesTest, IcebergConnectorValidatePropertiesTest, PaimonConnectorValidatePropertiesTest, MaxComputePartitionCacheTest, AdbcCatalogPropertiesTest; ExternalCatalogMetaCacheWeightTest - external_table_p0/test_catalog_ddl - Behavior changed: Yes. DDL that master accepted is now rejected: a misspelled option, a value of the wrong type, and a catalog or entry quota above the FE global quota. Compared with branch-4.x: unknown entries and engines a catalog does not route stay accepted; meta.cache.hive.partition_values.*, meta.cache.iceberg.snapshot.* and meta.cache.paimon.{table,snapshot}.max-weight are not supported on master (a submitted max-weight of them is rejected as an unknown weight entry, their other options and stored values are ignored); an entry max-weight of 0 is rejected instead of disabling the entry. - Does this need documentation: No 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.
Static review of all 12 changed files at 722469c found one P2 regression-test defect: a valid unit-bearing zero global quota makes the new expected-error case fail. No production-code defect was substantiated. There were no existing inline findings to carry forward, and no additional user focus was supplied.
Critical checkpoint conclusions:
- Goal and correctness: CREATE/ALTER validate submitted cache options/types and global weight bounds through shared CacheSpec helpers. Added unit and negative regression cases cover the intended behavior; the quota-test guard needs the inline correction.
- Scope and reuse: the change is focused on shared validation and the two catalog statement entry points, with a Remote Doris namespace override.
- Concurrency: concurrent catalog DDL retains detached read-locked snapshots and the write-locked identity/property retry fence. The new validation performs no I/O and introduces no locks or lock-order change.
- Lifecycle and initialization: Config/ExternalMetaCacheMgr initialize the shared parent-first governance bridge before statements; failed CREATE validation uses existing connector/context cleanup. No new resource lifetime or cross-TU static dependency is introduced.
- Configuration: no setting is added. The existing global quota is restart-only, and submitted-only comparisons preserve unrelated ALTER after a lowered quota.
- Compatibility: new submitted invalid values intentionally fail; persisted unknown keys are not checked again by the added validators. Runtime parsing and stored formats are unchanged; existing consumed-value checks retain their previous behavior.
- Thrift/protobuf: no definitions, field IDs or enum values change.
- Parallel paths: checked CREATE/ALTER/replay, core default and remote doris namespaces, Hive plus its Iceberg sibling, Paimon, MaxCompute and ADBC provider/runtime paths.
- Conditional checks: namespace, submitted-key and catalog-parent exemptions have explicit compatibility purposes. The regression's raw-string quota condition is the reported defect.
- Test coverage: inspected the added JUnit 5 option/type/bounds/compatibility tests and CREATE/ALTER negative regression cases. Builds and test execution were prohibited by this review contract and were not performed.
- Expected test results: no generated .out file changes. Error text matches validation/conversion paths, except the reported unlimited-quota scenario where no exception should occur.
- Observability and errors: validation fails explicitly with the offending key/value or weight bounds; existing DDL error boundaries remain intact. No new asynchronous path requires metrics.
- Persistence and failover: checks precede publication/journaling and are absent from replay; log formats and state transitions retain their existing paths.
- Data writes and atomicity: no table-data or transaction code changes. Catalog publication keeps existing locking/journaling, and rejected validation does not publish the candidate.
- FE/BE variable passing: no transmitted fields or send paths change.
- Performance and memory: work is linear in DDL property counts, with small temporary maps and no query-path or BE allocation change.
- Other invariants: committed-data visibility, partition versions, MoW delete bitmaps and BE nullable-column handling are outside this Java DDL-only change. No additional defect was substantiated in the final sweep.
All assigned static scopes and risk questions are resolved. Runtime behavior and test execution remain unverified under the static-only contract.
Review converged after 2 round(s).
|
|
||
| // A submitted weight may not exceed the FE global limit. Frontends run without one by default. | ||
| def globalWeight = sql """ADMIN SHOW FRONTEND CONFIG LIKE 'external_meta_cache_max_weight'""" | ||
| if (globalWeight.size() == 1 && globalWeight[0][1] != "0") { |
There was a problem hiding this comment.
[P2] Parse zero-valued quotas before enabling this assertion
external_meta_cache_max_weight=0B (or 0MB) is valid: ExternalMetaCacheMgr.configuredGlobalMaxWeight() parses it to zero and disables the cap, while ADMIN SHOW FRONTEND CONFIG returns the original string. This condition therefore enters the expected-error block even though meta.cache.max-weight=1PB is allowed, so test_catalog_ddl fails on an otherwise valid configuration. Determine whether the parsed quota is enabled before expecting CREATE to fail.
There was a problem hiding this comment.
The mechanism is right, but it cannot fire in CI: no pipeline or docker FE conf sets external_meta_cache_max_weight, so every pipeline uses the default "0" and skips this block (all pipelines are green on 722469c). The block is for a manual run against an FE that has a real quota. The suite is not meant to pass under every accepted value either: its earlier catalogs submit 128MB and 96MB, which this PR rejects when the global quota is smaller. Leaving the guard as is.
TPC-H: Total hot run time: 28393 ms |
TPC-DS: Total hot run time: 150301 ms |
ClickBench: Total hot run time: 24.31 s |
FE UT Coverage ReportIncrement line coverage |
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 DDL-time checks of
meta.cache.*catalog properties from branch-4.x #66717 to master. CREATE / ALTER CATALOG now rejects ameta.cache.<engine>.<entry>.<option>key that the statement submits for an engine the catalog owns when the option is notenable,ttl-second,capacityormax-weight, or when its value has the wrong type, and it rejects a catalog quota (or an entry quota of a catalog without one) above the FE global quotaexternal_meta_cache_max_weight. Unknown entries and keys stored by earlier statements stay accepted, as master intends.Background. External catalogs size their metadata caches with keys such as
meta.cache.hive.file.ttl-secondormeta.cache.iceberg.table.max-weight, plus a catalog quotameta.cache.max-weightthat is capped by the FE-wideexternal_meta_cache_max_weight. Master (#67726) rewrote the cache governance of branch-4.x (#66717) on the connector SPI. Branch-4.x validates everymeta.cache.*key at CREATE / ALTER against a whitelist. Master deliberately tolerates unknown entries, because a newer connector version may write an entry an older FE does not know, and it never re-validates stored keys, because ALTER can overwrite but not remove a key.The problem, and what it cost. Master only rejects an unknown
.max-weightentry that the statement submits and checks the values of a few keys, so these DDLs succeed:meta.cache.hive.file.max-weigth = 64MB: the limit the operator asked for is silently not applied;meta.cache.hive.file.enable = 1:Boolean.parseBooleanreads it as false, so the cache is silently turned off;meta.cache.hive.file.ttl-second = -2;meta.cache.max-weight = 1PBon an FE whose global quota is 1GB: admission clamps the caches to the global quota at run time, so SHOW CREATE CATALOG promises far more memory than the caches can get.How this PR fixes it.
CacheSpec.checkSubmittedEnginePropertieschecks the keys a statement submits undermeta.cache.<engine>.: the option after the last dot must beenable(true / false),ttl-second(>= -1),capacity(>= 0) ormax-weight(a positive size); anything else fails withUnknown external meta cache property: <key>, the branch-4.x message. The value messages are the existing ones (The parameter <key> is wrong, value is <value>,Invalid cache weight for '<key>': <value>). Entry names are not checked.CacheSpec.checkSubmittedWeightsAgainstGlobalrejects a submittedmeta.cache.max-weight, and a submitted entrymax-weightof a catalog without a catalog quota, above the global quota of the FE running the statement (nothing is checked when the global quota is unset).CacheSpec.checkWeightProperties(properties, submittedProperties, engine, ...), which the hive connector (forhiveand itsicebergsibling), iceberg, paimon, max_compute and adbc already call with the statement's properties, so no connector changes. For the namespace that fe-core owns (default, ordorisfor a remote Doris catalog), a newExternalCatalog.checkSubmittedMetaCachePropertieshook runs them;CatalogMgrcalls it on CREATE and ALTER CATALOG only, never on replay.Differences from branch-4.x, kept on purpose:
meta.cache.iceberg.schema.ttl-second) and a key of an engine the catalog does not route stay accepted and are ignored.meta.cache.hive.partition_values.*,meta.cache.iceberg.snapshot.*andmeta.cache.paimon.{table,snapshot}.max-weightare not supported on master: a submittedmax-weightof them is rejected as an unknown weight entry, their other options are accepted and ignored, and stored values are ignored. A catalog that only had entry quotas on them has no weight limit after upgrading.max-weight = 0is rejected; a stored 0 means no entry limit rather than a disabled entry.Results.
meta.cache.hive.file.max-weigth = 64MBUnknown external meta cache property: meta.cache.hive.file.max-weigthmeta.cache.hive.file.enable = 1The parameter meta.cache.hive.file.enable is wrong, value is 1meta.cache.iceberg.table.ttl-second = -2on an hms catalogThe parameter meta.cache.iceberg.table.ttl-second is wrong, value is -2ALTER ... meta.cache.default.schema.enable = 1meta.cache.max-weight = 1PBwith a 1GB global quotameta.cache.max-weight (1125899906842624 bytes) can not exceed the FE global max weight external_meta_cache_max_weight (1073741824 bytes)meta.cache.default.future_entry.max-weight = 64MB, then an unrelated ALTERUnit tests:
CacheSpecTest(3 new), one new test each inHiveCatalogPropertiesTest,IcebergConnectorValidatePropertiesTest,PaimonConnectorValidatePropertiesTest,MaxComputePartitionCacheTestandAdbcCatalogPropertiesTest, and 3 new tests inExternalCatalogMetaCacheWeightTest(the fe-core hook, the global quota passed in explicitly, and the remote Doris engine). Without the submitted-key check the new hive, iceberg and paimon tests fail; without the global-quota check, or without the remote Doris engine, the corresponding tests fail. All tests of fe-connector-cache, -hive, -iceberg, -paimon, -maxcompute and -adbc pass, and so do 280 fe-core tests around catalog DDL and the metadata caches.external_table_p0/test_catalog_ddlgains the misspelled option,enable=1,ttl-second=-2and the ALTER case; its global-quota case runs only whenexternal_meta_cache_max_weightis set, since CI frontends run without one. The suite passed on a local cluster both without a global quota and withexternal_meta_cache_max_weight = 1GB.Release note
CREATE / ALTER CATALOG now rejects a
meta.cache.<engine>.<entry>.<option>property whose option is notenable,ttl-second,capacityormax-weightor whose value has the wrong type, and ameta.cache.max-weight(or an entrymax-weightof a catalog without one) aboveexternal_meta_cache_max_weight.Check List (For Author)
Test
Behavior changed:
DDL that master accepted is now rejected: a misspelled option, a value of the wrong type, and a catalog or entry quota above the FE global quota. The differences from branch-4.x are listed above.
Does this need documentation?
Check List (For Reviewer who merge this PR)