Skip to content

[improvement](fe) Validate submitted meta.cache keys of the catalog's own engines - #69556

Merged
morningman merged 1 commit into
apache:masterfrom
morningman:b2m-t12c-meta-cache-strict-keys
Oct 25, 2026
Merged

morningman merged 1 commit into
apache:masterfrom
morningman:b2m-t12c-meta-cache-strict-keys

Conversation

@morningman

Copy link
Copy Markdown
Contributor

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 a meta.cache.<engine>.<entry>.<option> key that the statement submits for an engine the catalog owns when the option is not enable, ttl-second, capacity or max-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 quota external_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-second or meta.cache.iceberg.table.max-weight, plus a catalog quota meta.cache.max-weight that is capped by the FE-wide external_meta_cache_max_weight. Master (#67726) rewrote the cache governance of branch-4.x (#66717) on the connector SPI. Branch-4.x validates every meta.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-weight entry that the statement submits and checks the values of a few keys, so these DDLs succeed:

  • a misspelled option such as meta.cache.hive.file.max-weigth = 64MB: the limit the operator asked for is silently not applied;
  • meta.cache.hive.file.enable = 1: Boolean.parseBoolean reads it as false, so the cache is silently turned off;
  • meta.cache.hive.file.ttl-second = -2;
  • meta.cache.max-weight = 1PB on 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.checkSubmittedEngineProperties checks the keys a statement submits under meta.cache.<engine>.: the option after the last dot must be enable (true / false), ttl-second (>= -1), capacity (>= 0) or max-weight (a positive size); anything else fails with Unknown 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.checkSubmittedWeightsAgainstGlobal rejects a submitted meta.cache.max-weight, and a submitted entry max-weight of 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).
  • Both run inside CacheSpec.checkWeightProperties(properties, submittedProperties, engine, ...), which the hive connector (for hive and its iceberg sibling), 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, or doris for a remote Doris catalog), a new ExternalCatalog.checkSubmittedMetaCacheProperties hook runs them; CatalogMgr calls it on CREATE and ALTER CATALOG only, never on replay.
  • Only submitted keys are checked, so a key stored by an earlier statement, or a stored quota above a global quota lowered after a restart, never blocks an unrelated ALTER.

Differences from branch-4.x, kept on purpose:

  • An unknown entry (for example 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.* 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 are accepted and ignored, and stored values are ignored. A catalog that only had entry quotas on them has no weight limit after upgrading.
  • An entry max-weight = 0 is rejected; a stored 0 means no entry limit rather than a disabled entry.

Results.

DDL Master today After this PR
meta.cache.hive.file.max-weigth = 64MB accepted, ignored Unknown external meta cache property: meta.cache.hive.file.max-weigth
meta.cache.hive.file.enable = 1 accepted, cache off The parameter meta.cache.hive.file.enable is wrong, value is 1
meta.cache.iceberg.table.ttl-second = -2 on an hms catalog accepted The parameter meta.cache.iceberg.table.ttl-second is wrong, value is -2
ALTER ... meta.cache.default.schema.enable = 1 accepted rejected the same way
meta.cache.max-weight = 1PB with a 1GB global quota accepted, clamped at run time meta.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 ALTER accepted accepted (unchanged)

Unit tests: CacheSpecTest (3 new), one new test each in HiveCatalogPropertiesTest, IcebergConnectorValidatePropertiesTest, PaimonConnectorValidatePropertiesTest, MaxComputePartitionCacheTest and AdbcCatalogPropertiesTest, and 3 new tests in ExternalCatalogMetaCacheWeightTest (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_ddl gains the misspelled option, enable=1, ttl-second=-2 and the ALTER case; its global-quota case runs only when external_meta_cache_max_weight is set, since CI frontends run without one. The suite passed on a local cluster both without a global quota and with external_meta_cache_max_weight = 1GB.

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

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • 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. The differences from branch-4.x are listed above.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

… 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]>
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@morningman

Copy link
Copy Markdown
Contributor Author

run buildall

@morningman

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. 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.
  2. Scope and reuse: the change is focused on shared validation and the two catalog statement entry points, with a Remote Doris namespace override.
  3. 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.
  4. 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.
  5. Configuration: no setting is added. The existing global quota is restart-only, and submitted-only comparisons preserve unrelated ALTER after a lowered quota.
  6. 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.
  7. Thrift/protobuf: no definitions, field IDs or enum values change.
  8. Parallel paths: checked CREATE/ALTER/replay, core default and remote doris namespaces, Hive plus its Iceberg sibling, Paimon, MaxCompute and ADBC provider/runtime paths.
  9. Conditional checks: namespace, submitted-key and catalog-parent exemptions have explicit compatibility purposes. The regression's raw-string quota condition is the reported defect.
  10. 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.
  11. 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.
  12. 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.
  13. Persistence and failover: checks precede publication/journaling and are absent from replay; log formats and state transitions retain their existing paths.
  14. 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.
  15. FE/BE variable passing: no transmitted fields or send paths change.
  16. Performance and memory: work is linear in DDL property counts, with small temporary maps and no query-path or BE allocation change.
  17. 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") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28393 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 722469cf3de73e68b06012e23c2d54d262a4081b, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17598	5214	5068	5068
q2	2135	298	260	260
q3	10245	1358	821	821
q4	4681	461	336	336
q5	7425	808	526	526
q6	179	182	150	150
q7	761	806	599	599
q8	9264	1315	1015	1015
q9	5523	4186	4156	4156
q10	6827	1282	1024	1024
q11	435	246	225	225
q12	635	392	282	282
q13	18089	3562	3004	3004
q14	268	269	245	245
q15	q16	719	718	661	661
q17	1174	748	591	591
q18	6996	6174	6090	6090
q19	1114	947	593	593
q20	383	337	220	220
q21	5472	2269	2524	2269
q22	374	326	258	258
Total cold run time: 100297 ms
Total hot run time: 28393 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5949	5978	5956	5956
q2	700	589	569	569
q3	5527	5668	5236	5236
q4	1996	2164	1529	1529
q5	5861	5590	5709	5590
q6	248	197	159	159
q7	2317	2040	1817	1817
q8	3007	2653	2616	2616
q9	7602	7484	7464	7464
q10	3643	3571	3124	3124
q11	596	422	376	376
q12	660	653	491	491
q13	3245	3555	3048	3048
q14	276	279	262	262
q15	q16	680	703	626	626
q17	7691	7005	6861	6861
q18	12968	12168	12769	12168
q19	918	795	764	764
q20	2163	2130	1938	1938
q21	5530	4568	4658	4568
q22	511	471	426	426
Total cold run time: 72088 ms
Total hot run time: 65588 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 150301 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 722469cf3de73e68b06012e23c2d54d262a4081b, data reload: false

query5	4307	577	440	440
query6	434	208	188	188
query7	4811	437	219	219
query8	329	164	151	151
query9	8784	3092	3041	3041
query10	483	302	238	238
query11	5859	3847	3573	3573
query12	154	94	83	83
query13	1251	441	316	316
query14	6099	3951	3611	3611
query14_1	3454	3422	3413	3413
query15	204	194	180	180
query16	950	422	425	422
query17	859	654	513	513
query18	2393	414	299	299
query19	197	159	130	130
query20	81	81	87	81
query21	212	126	105	105
query22	12929	13061	12941	12941
query23	13434	12770	12432	12432
query23_1	12540	12377	12505	12377
query24	7362	814	456	456
query24_1	466	464	462	462
query25	495	388	334	334
query26	1237	252	131	131
query27	2792	445	269	269
query28	5381	1924	1984	1924
query29	1566	587	434	434
query30	300	212	185	185
query31	1034	935	818	818
query32	147	92	83	83
query33	520	283	227	227
query34	874	846	499	499
query35	745	750	664	664
query36	799	814	724	724
query37	127	104	82	82
query38	1764	1741	1681	1681
query39	798	806	749	749
query39_1	727	723	738	723
query40	217	116	95	95
query41	71	68	66	66
query42	87	85	88	85
query43	345	363	309	309
query44	1315	685	688	685
query45	178	194	169	169
query46	829	979	572	572
query47	2932	2971	2930	2930
query48	294	331	220	220
query49	589	403	302	302
query50	677	271	208	208
query51	10697	10601	10410	10410
query52	83	79	70	70
query53	189	208	150	150
query54	249	204	198	198
query55	73	72	72	72
query56	240	224	219	219
query57	1662	1594	1570	1570
query58	293	275	260	260
query59	2073	2135	1920	1920
query60	295	249	233	233
query61	167	198	161	161
query62	386	344	284	284
query63	181	155	170	155
query64	2736	933	763	763
query65	3361	3266	3292	3266
query66	1779	421	290	290
query67	20051	20050	19750	19750
query68	2914	1001	611	611
query69	386	283	255	255
query70	830	757	747	747
query71	299	238	205	205
query72	2768	2619	2312	2312
query73	517	524	297	297
query74	4617	4476	4352	4352
query75	2414	2272	1898	1898
query76	2315	995	582	582
query77	343	382	262	262
query78	9552	9645	9137	9137
query79	922	852	523	523
query80	547	403	335	335
query81	530	327	263	263
query82	584	132	100	100
query83	303	195	179	179
query84	317	128	101	101
query85	862	470	377	377
query86	329	237	230	230
query87	2014	1920	1801	1801
query88	3651	2673	2656	2656
query89	328	280	238	238
query90	1857	166	170	166
query91	162	153	129	129
query92	100	87	88	87
query93	1005	1006	582	582
query94	437	272	264	264
query95	592	400	297	297
query96	655	483	230	230
query97	2441	2398	2348	2348
query98	163	157	143	143
query99	771	770	648	648
Total cold run time: 234061 ms
Total hot run time: 150301 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.31 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 722469cf3de73e68b06012e23c2d54d262a4081b, data reload: false

query1	0.00	0.00	0.00
query2	0.10	0.05	0.05
query3	0.26	0.09	0.08
query4	1.62	0.10	0.10
query5	0.27	0.24	0.25
query6	1.17	0.70	0.69
query7	0.04	0.00	0.01
query8	0.06	0.04	0.04
query9	0.40	0.35	0.36
query10	0.61	0.58	0.57
query11	0.19	0.14	0.14
query12	0.19	0.16	0.15
query13	0.49	0.50	0.50
query14	1.03	1.02	1.02
query15	0.65	0.62	0.60
query16	0.36	0.36	0.34
query17	1.17	1.15	1.16
query18	0.23	0.22	0.21
query19	2.14	1.92	1.98
query20	0.02	0.02	0.01
query21	15.48	0.20	0.14
query22	4.89	0.06	0.06
query23	16.28	0.32	0.13
query24	3.19	0.58	0.39
query25	0.11	0.05	0.06
query26	0.78	0.22	0.15
query27	0.05	0.03	0.05
query28	3.46	0.82	0.39
query29	12.48	4.13	3.26
query30	0.28	0.13	0.10
query31	2.77	0.56	0.32
query32	3.23	0.60	0.50
query33	3.13	3.16	3.20
query34	15.32	3.99	3.39
query35	3.36	3.30	3.35
query36	0.56	0.43	0.44
query37	0.10	0.06	0.07
query38	0.06	0.04	0.04
query39	0.04	0.03	0.02
query40	0.17	0.15	0.14
query41	0.08	0.03	0.03
query42	0.04	0.03	0.03
query43	0.04	0.04	0.03
Total cold run time: 96.9 s
Total hot run time: 24.31 s

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 94.74% (18/19) 🎉
Increment coverage report
Complete coverage report

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 94.74% (18/19) 🎉
Increment coverage report
Complete coverage report

@morningman
morningman merged commit af847e7 into apache:master Oct 25, 2026
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants