morningman opened a new pull request, #69556:
URL: https://github.com/apache/doris/pull/69556
### 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 glob
al 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 <!-- At least one of them must be included. -->
- [x] Regression test
- [x] 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 <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
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?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
