This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new af847e7a4d4 [improvement](fe) Validate submitted meta.cache keys of
the catalog's own engines (#69556)
af847e7a4d4 is described below
commit af847e7a4d4e6c94017e734a17fe3b7e0676ff01
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Sun Oct 25 08:04:05 2026 +0000
[improvement](fe) Validate submitted meta.cache keys of the catalog's own
engines (#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 global quota and with `external_meta_cache_max_weight = 1GB`.
---
.../connector/adbc/AdbcCatalogPropertiesTest.java | 13 +++
.../apache/doris/connector/cache/CacheSpec.java | 97 ++++++++++++++++++++--
.../doris/connector/cache/CacheSpecTest.java | 81 ++++++++++++++++++
.../connector/hive/HiveCatalogPropertiesTest.java | 18 ++++
.../IcebergConnectorValidatePropertiesTest.java | 11 +++
.../maxcompute/MaxComputePartitionCacheTest.java | 15 ++++
.../PaimonConnectorValidatePropertiesTest.java | 15 ++++
.../org/apache/doris/datasource/CatalogMgr.java | 8 ++
.../apache/doris/datasource/ExternalCatalog.java | 31 ++++++-
.../doris/RemoteDorisExternalCatalog.java | 5 ++
.../ExternalCatalogMetaCacheWeightTest.java | 56 +++++++++++++
.../external_table_p0/test_catalog_ddl.groovy | 54 ++++++++++++
12 files changed, 398 insertions(+), 6 deletions(-)
diff --git
a/fe/fe-connector/fe-connector-adbc/src/test/java/org/apache/doris/connector/adbc/AdbcCatalogPropertiesTest.java
b/fe/fe-connector/fe-connector-adbc/src/test/java/org/apache/doris/connector/adbc/AdbcCatalogPropertiesTest.java
index c3ecbfd8ab8..764499aeb00 100644
---
a/fe/fe-connector/fe-connector-adbc/src/test/java/org/apache/doris/connector/adbc/AdbcCatalogPropertiesTest.java
+++
b/fe/fe-connector/fe-connector-adbc/src/test/java/org/apache/doris/connector/adbc/AdbcCatalogPropertiesTest.java
@@ -249,6 +249,19 @@ class AdbcCatalogPropertiesTest {
current, Map.of("meta.cache.max-weight", "1MB",
"meta.cache.adbc.metadata.max-weight", "2MB")));
}
+ @Test
+ void submittedMetaCacheKeysMustNameAKnownOption() {
+ AdbcConnectorProvider provider = new AdbcConnectorProvider();
+ Map<String, String> current = minimal();
+ IllegalArgumentException misspelled =
Assertions.assertThrows(IllegalArgumentException.class,
+ () -> provider.validatePropertiesForUpdate(
+ current, Map.of("meta.cache.adbc.metadata.ttl-secnod",
"60")));
+ Assertions.assertEquals("Unknown external meta cache property:
meta.cache.adbc.metadata.ttl-secnod",
+ misspelled.getMessage());
+ current.put("meta.cache.adbc.metadata.ttl-secnod", "60");
+ Assertions.assertDoesNotThrow(() ->
provider.validatePropertiesForUpdate(current, Map.of("comment", "new")));
+ }
+
/**
* A cache knob is read through {@code CacheSpec}, which answers an
unparseable value with the default
* rather than an error. Silently caching for ten minutes when the
operator wrote {@code ttl-second=6O0}
diff --git
a/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/CacheSpec.java
b/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/CacheSpec.java
index c720139716e..f3f2150159a 100644
---
a/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/CacheSpec.java
+++
b/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/CacheSpec.java
@@ -220,13 +220,26 @@ public final class CacheSpec {
checkWeightProperties(properties, Collections.emptyMap(), engine,
knownEntries);
}
- /** DDL validation: reject unknown entries only when explicitly submitted
by this statement. */
+ /**
+ * DDL validation: the keys this statement submits for {@code engine} must
be well formed (see
+ * {@link #checkSubmittedEngineProperties}), its submitted weights must
not exceed the FE-global limit, and an
+ * unknown weight entry is rejected only when this statement submits it.
+ */
public static void checkWeightProperties(Map<String, String> properties,
Map<String, String> submittedProperties, String engine, String...
knownEntries) {
+ checkWeightProperties(properties, submittedProperties,
MetaCacheGovernance.globalMaxWeight(), engine,
+ knownEntries);
+ }
+
+ /** As {@link #checkWeightProperties(Map, Map, String, String...)},
against the given FE-global limit. */
+ public static void checkWeightProperties(Map<String, String> properties,
Map<String, String> submittedProperties,
+ OptionalLong globalMaxWeight, String engine, String...
knownEntries) {
+ checkSubmittedEngineProperties(submittedProperties, engine);
OptionalLong catalogMax = checkCatalogWeightProperty(properties);
if (properties == null) {
return;
}
+ checkSubmittedWeightsAgainstGlobal(properties, submittedProperties,
globalMaxWeight, engine);
String prefix = metaCacheKeyPrefix(engine);
Set<String> weightKeys = new HashSet<>();
for (String entry : knownEntries) {
@@ -244,10 +257,7 @@ public final class CacheSpec {
// Like other connector properties, validate consumed values
but tolerate unknown keys.
continue;
}
- long parsed = parseWeight(property.getValue(), key, false, 0L);
- if (parsed <= 0L) {
- throw new IllegalArgumentException(key + " must be positive");
- }
+ long parsed = parsePositiveWeight(property.getValue(), key);
if (catalogMax.isPresent() && parsed > catalogMax.getAsLong()) {
throw new IllegalArgumentException(key + " can not exceed "
+ MetaCacheBudgetManager.CATALOG_MAX_WEIGHT_PROPERTY);
@@ -255,6 +265,71 @@ public final class CacheSpec {
}
}
+ /**
+ * DDL validation of the {@code meta.cache.<engine>.<entry>.<option>} keys
this statement submits: the option must
+ * be enable, ttl-second, capacity or max-weight, and the value one of its
type. Entry names are not checked, so a
+ * key of an entry that a newer connector reads stays accepted, and keys
an earlier statement persisted are not
+ * inspected, so a stored key never blocks an unrelated ALTER.
+ */
+ public static void checkSubmittedEngineProperties(Map<String, String>
submittedProperties, String engine) {
+ String prefix = metaCacheKeyPrefix(engine);
+ for (Map.Entry<String, String> property :
submittedProperties.entrySet()) {
+ String key = property.getKey();
+ if (!key.startsWith(prefix)) {
+ continue;
+ }
+ int optionStart = key.lastIndexOf('.');
+ if (optionStart <= prefix.length()) {
+ throw unknownProperty(key);
+ }
+ String value = property.getValue();
+ switch (key.substring(optionStart)) {
+ case KEY_ENABLE:
+ checkBooleanProperty(value, key);
+ break;
+ case KEY_TTL_SECOND:
+ checkLongProperty(value, CACHE_NO_TTL, key);
+ break;
+ case KEY_CAPACITY:
+ checkLongProperty(value, 0L, key);
+ break;
+ case KEY_MAX_WEIGHT:
+ parsePositiveWeight(value, key);
+ break;
+ default:
+ throw unknownProperty(key);
+ }
+ }
+ }
+
+ /**
+ * DDL validation against the FE-global limit, the one of the FE running
the statement: a submitted catalog
+ * weight, and a submitted {@code engine} entry weight of a catalog
without a catalog weight, must not exceed it.
+ * Admission clamps every cache to the limit anyway; this keeps SHOW
CREATE CATALOG from promising more. Only
+ * submitted keys are checked, so a limit lowered after a restart never
blocks an unrelated ALTER.
+ */
+ public static void checkSubmittedWeightsAgainstGlobal(Map<String, String>
effectiveProperties,
+ Map<String, String> submittedProperties, OptionalLong
globalMaxWeight, String engine) {
+ if (!globalMaxWeight.isPresent()) {
+ return;
+ }
+ boolean catalogLimited =
effectiveProperties.containsKey(MetaCacheBudgetManager.CATALOG_MAX_WEIGHT_PROPERTY);
+ String prefix = metaCacheKeyPrefix(engine);
+ for (Map.Entry<String, String> property :
submittedProperties.entrySet()) {
+ String key = property.getKey();
+ boolean limitedByGlobal =
key.equals(MetaCacheBudgetManager.CATALOG_MAX_WEIGHT_PROPERTY)
+ || !catalogLimited && key.startsWith(prefix) &&
key.endsWith(KEY_MAX_WEIGHT);
+ if (!limitedByGlobal) {
+ continue;
+ }
+ long weight = parsePositiveWeight(property.getValue(), key);
+ if (weight > globalMaxWeight.getAsLong()) {
+ throw new IllegalArgumentException(key + " (" + weight + "
bytes) can not exceed the FE global max"
+ + " weight external_meta_cache_max_weight (" +
globalMaxWeight.getAsLong() + " bytes)");
+ }
+ }
+ }
+
public static boolean isCacheEnabled(boolean enable, long ttlSecond, long
capacity) {
return enable && ttlSecond != 0 && capacity != 0;
}
@@ -400,6 +475,18 @@ public final class CacheSpec {
return value.longValue();
}
+ private static long parsePositiveWeight(String value, String key) {
+ long parsed = parseWeight(value, key, false, 0L);
+ if (parsed <= 0L) {
+ throw new IllegalArgumentException(key + " must be positive");
+ }
+ return parsed;
+ }
+
+ private static IllegalArgumentException unknownProperty(String key) {
+ return new IllegalArgumentException("Unknown external meta cache
property: " + key);
+ }
+
private static IllegalArgumentException invalidWeight(String key, String
value) {
return new IllegalArgumentException("Invalid cache weight for '" + key
+ "': " + value);
}
diff --git
a/fe/fe-connector/fe-connector-cache/src/test/java/org/apache/doris/connector/cache/CacheSpecTest.java
b/fe/fe-connector/fe-connector-cache/src/test/java/org/apache/doris/connector/cache/CacheSpecTest.java
index 1169a4ef505..4d5080a2516 100644
---
a/fe/fe-connector/fe-connector-cache/src/test/java/org/apache/doris/connector/cache/CacheSpecTest.java
+++
b/fe/fe-connector/fe-connector-cache/src/test/java/org/apache/doris/connector/cache/CacheSpecTest.java
@@ -285,6 +285,87 @@ public class CacheSpecTest {
() -> CacheSpec.checkCatalogWeightProperty(properties));
}
+ @Test
+ public void submittedEngineKeysMustNameAKnownOptionWithAValueOfItsType() {
+ IllegalArgumentException misspelled =
Assertions.assertThrows(IllegalArgumentException.class,
+ () -> CacheSpec.checkSubmittedEngineProperties(
+
Collections.singletonMap("meta.cache.iceberg.table.max-weigth", "16MB"),
"iceberg"));
+ Assertions.assertEquals("Unknown external meta cache property:
meta.cache.iceberg.table.max-weigth",
+ misspelled.getMessage());
+ for (String key : new String[] {"meta.cache.iceberg.max-weight",
"meta.cache.iceberg.enable",
+ "meta.cache.iceberg..enable", "meta.cache.iceberg.table."}) {
+ Assertions.assertThrows(IllegalArgumentException.class, () ->
CacheSpec.checkSubmittedEngineProperties(
+ Collections.singletonMap(key, "1"), "iceberg"), key);
+ }
+ IllegalArgumentException enable =
Assertions.assertThrows(IllegalArgumentException.class,
+ () -> CacheSpec.checkSubmittedEngineProperties(
+
Collections.singletonMap("meta.cache.iceberg.table.enable", "1"), "iceberg"));
+ Assertions.assertEquals("The parameter meta.cache.iceberg.table.enable
is wrong, value is 1",
+ enable.getMessage());
+ IllegalArgumentException ttl =
Assertions.assertThrows(IllegalArgumentException.class,
+ () -> CacheSpec.checkSubmittedEngineProperties(
+
Collections.singletonMap("meta.cache.iceberg.table.ttl-second", "-2"),
"iceberg"));
+ Assertions.assertEquals("The parameter
meta.cache.iceberg.table.ttl-second is wrong, value is -2",
+ ttl.getMessage());
+ Assertions.assertThrows(IllegalArgumentException.class, () ->
CacheSpec.checkSubmittedEngineProperties(
+
Collections.singletonMap("meta.cache.iceberg.manifest.capacity", "-1"),
"iceberg"));
+ Assertions.assertThrows(IllegalArgumentException.class, () ->
CacheSpec.checkSubmittedEngineProperties(
+
Collections.singletonMap("meta.cache.iceberg.manifest.max-weight", "0"),
"iceberg"));
+
+ // Entries are not checked: a newer connector may read one this
version does not know. Other engines'
+ // keys, the catalog weight and plain properties belong to other
validators.
+ Map<String, String> accepted = new HashMap<>();
+ accepted.put("meta.cache.iceberg.schema.ttl-second", "0");
+ accepted.put("meta.cache.iceberg.future_entry.enable", "TRUE");
+ accepted.put("meta.cache.iceberg.table.ttl-second", "-1");
+ accepted.put("meta.cache.iceberg.table.capacity", "0");
+ accepted.put("meta.cache.iceberg.table.max-weight", "64MB");
+ accepted.put("meta.cache.paimon.table.enable", "1");
+ accepted.put("meta.cache.max-weight", "1GB");
+ accepted.put("comment", "x");
+ CacheSpec.checkSubmittedEngineProperties(accepted, "iceberg");
+ }
+
+ @Test
+ public void checkWeightPropertiesValidatesOnlyTheSubmittedEngineKeys() {
+ Map<String, String> stored = new HashMap<>();
+ stored.put("meta.cache.iceberg.table.max-weigth", "16MB");
+ stored.put("meta.cache.iceberg.table.enable", "1");
+ Assertions.assertDoesNotThrow(() -> CacheSpec.checkWeightProperties(
+ stored, Collections.singletonMap("comment", "updated"),
"iceberg", "table"));
+ Assertions.assertThrows(IllegalArgumentException.class, () ->
CacheSpec.checkWeightProperties(
+ stored,
Collections.singletonMap("meta.cache.iceberg.table.enable", "yes"), "iceberg",
"table"));
+ }
+
+ @Test
+ public void submittedWeightsMayNotExceedTheGlobalLimit() {
+ OptionalLong global = OptionalLong.of(1024L * 1024 * 1024);
+ Map<String, String> catalogAbove =
Collections.singletonMap("meta.cache.max-weight", "2GB");
+ IllegalArgumentException e =
Assertions.assertThrows(IllegalArgumentException.class,
+ () -> CacheSpec.checkWeightProperties(catalogAbove,
catalogAbove, global, "iceberg", "table"));
+ Assertions.assertEquals("meta.cache.max-weight (2147483648 bytes) can
not exceed the FE global max weight"
+ + " external_meta_cache_max_weight (1073741824 bytes)",
e.getMessage());
+ Map<String, String> catalogAtLimit =
Collections.singletonMap("meta.cache.max-weight", "1GB");
+ CacheSpec.checkWeightProperties(catalogAtLimit, catalogAtLimit,
global, "iceberg", "table");
+
+ // Without a catalog weight the global limit bounds an entry weight;
with one the catalog weight does.
+ Map<String, String> entryAbove =
Collections.singletonMap("meta.cache.iceberg.table.max-weight", "2GB");
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () -> CacheSpec.checkWeightProperties(entryAbove, entryAbove,
global, "iceberg", "table"));
+ Map<String, String> limitedCatalog = new HashMap<>(entryAbove);
+ limitedCatalog.put("meta.cache.max-weight", "1GB");
+ IllegalArgumentException parent =
Assertions.assertThrows(IllegalArgumentException.class,
+ () -> CacheSpec.checkWeightProperties(limitedCatalog,
entryAbove, global, "iceberg", "table"));
+ Assertions.assertEquals("meta.cache.iceberg.table.max-weight can not
exceed meta.cache.max-weight",
+ parent.getMessage());
+
+ // A stored weight is not compared again, e.g. after the limit was
lowered, and no limit bounds nothing.
+ CacheSpec.checkWeightProperties(catalogAbove,
Collections.singletonMap("comment", "updated"), global,
+ "iceberg", "table");
+ CacheSpec.checkWeightProperties(catalogAbove, catalogAbove,
OptionalLong.empty(), "iceberg", "table");
+ CacheSpec.checkSubmittedWeightsAgainstGlobal(entryAbove, entryAbove,
global, "paimon");
+ }
+
@Test
public void invalidPersistedEntryWeightFallsBackToUnbounded() {
Map<String, String> properties = new HashMap<>();
diff --git
a/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveCatalogPropertiesTest.java
b/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveCatalogPropertiesTest.java
index ef81f1b6e1e..73df0bb4966 100644
---
a/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveCatalogPropertiesTest.java
+++
b/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveCatalogPropertiesTest.java
@@ -245,6 +245,24 @@ class HiveCatalogPropertiesTest {
HiveTestProperties.mapWith(), unknownEntry));
}
+ @Test
+ void submittedMetaCacheKeysOfBothEnginesMustNameAKnownOption() {
+ HiveConnectorProvider provider = new HiveConnectorProvider();
+ IllegalArgumentException misspelled =
Assertions.assertThrows(IllegalArgumentException.class,
+ () -> provider.validateProperties(
+
HiveTestProperties.mapWith("meta.cache.hive.file.max-weigth", "64MB")));
+ Assertions.assertEquals("Unknown external meta cache property:
meta.cache.hive.file.max-weigth",
+ misspelled.getMessage());
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () ->
provider.validateProperties(HiveTestProperties.mapWith("meta.cache.hive.file.enable",
"1")));
+ Assertions.assertThrows(IllegalArgumentException.class, () ->
provider.validateProperties(
+
HiveTestProperties.mapWith("meta.cache.iceberg.table.ttl-second", "-2")));
+ // A key stored by an earlier statement is not validated again by an
unrelated ALTER.
+ Assertions.assertDoesNotThrow(() ->
provider.validatePropertiesForUpdate(
+ HiveTestProperties.mapWith("meta.cache.hive.file.max-weigth",
"64MB"),
+ java.util.Collections.singletonMap("comment", "updated")));
+ }
+
@Test
void validCatalogPassesBothDoors() {
Assertions.assertDoesNotThrow(() ->
HiveTestProperties.minimal().checkCreateTimeOnlyRules());
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorValidatePropertiesTest.java
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorValidatePropertiesTest.java
index 7e2d2268c44..7241b620a39 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorValidatePropertiesTest.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorValidatePropertiesTest.java
@@ -130,6 +130,17 @@ public class IcebergConnectorValidatePropertiesTest {
"meta.cache.iceberg.table.enable", "maybe")).contains("is
wrong"));
}
+ @Test
+ public void misspelledMetaCacheOptionRejectedThroughProvider() {
+ // The option must be known; an unknown entry stays accepted because a
newer connector may read it.
+ Assertions.assertEquals(
+ "Unknown external meta cache property:
meta.cache.iceberg.table.max-weigth",
+ rejectMessage(props("iceberg.catalog.type", "hms",
"hive.metastore.uris", "thrift://h",
+ "meta.cache.iceberg.table.max-weigth", "16MB")));
+ PROVIDER.validateProperties(props("iceberg.catalog.type", "hms",
"hive.metastore.uris", "thrift://h",
+ "meta.cache.iceberg.schema.ttl-second", "0"));
+ }
+
@Test
public void validMetaCacheKnobsAcceptedThroughProvider() {
// ttl-second=-1 (no-expiration sentinel, min is -1), 0 (disable),
capacity=0, boolean enable all pass.
diff --git
a/fe/fe-connector/fe-connector-maxcompute/src/test/java/org/apache/doris/connector/maxcompute/MaxComputePartitionCacheTest.java
b/fe/fe-connector/fe-connector-maxcompute/src/test/java/org/apache/doris/connector/maxcompute/MaxComputePartitionCacheTest.java
index d1245b6a12c..61b8d2acaf7 100644
---
a/fe/fe-connector/fe-connector-maxcompute/src/test/java/org/apache/doris/connector/maxcompute/MaxComputePartitionCacheTest.java
+++
b/fe/fe-connector/fe-connector-maxcompute/src/test/java/org/apache/doris/connector/maxcompute/MaxComputePartitionCacheTest.java
@@ -91,6 +91,21 @@ public class MaxComputePartitionCacheTest {
current, Map.of("meta.cache.max-weight", "1MB",
"meta.cache.max_compute.partition.max-weight", "2MB")));
}
+ @Test
+ public void submittedMetaCacheKeysMustNameAKnownOption() {
+ MaxComputeConnectorProvider provider = new
MaxComputeConnectorProvider();
+ Map<String, String> current = MCTestProperties.minimalMap();
+ IllegalArgumentException misspelled =
Assertions.assertThrows(IllegalArgumentException.class,
+ () -> provider.validatePropertiesForUpdate(
+ current,
Map.of("meta.cache.max_compute.partition.max-weigth", "1MB")));
+ Assertions.assertEquals("Unknown external meta cache property:
meta.cache.max_compute.partition.max-weigth",
+ misspelled.getMessage());
+ Assertions.assertThrows(IllegalArgumentException.class, () ->
provider.validatePropertiesForUpdate(
+ current, Map.of("meta.cache.max_compute.partition.enable",
"1")));
+ current.put("meta.cache.max_compute.partition.max-weigth", "1MB");
+ Assertions.assertDoesNotThrow(() ->
provider.validatePropertiesForUpdate(current, Map.of("comment", "new")));
+ }
+
@Test
public void managedCacheRetainsOnlySpecsAndHonorsBothLimits() throws
Exception {
Odps odps = new Odps(new AliyunAccount("ak", "sk"));
diff --git
a/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorValidatePropertiesTest.java
b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorValidatePropertiesTest.java
index 991cdffd535..96c28cf3019 100644
---
a/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorValidatePropertiesTest.java
+++
b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorValidatePropertiesTest.java
@@ -98,6 +98,21 @@ public class PaimonConnectorValidatePropertiesTest {
"meta.cache.paimon.table.enable", "maybe")));
}
+ @Test
+ public void rejectsMisspelledMetaCacheOption() {
+ IllegalArgumentException misspelled =
Assertions.assertThrows(IllegalArgumentException.class,
+ () -> validate(props(
+ "paimon.catalog.type", "filesystem", "warehouse",
"/wh",
+ "meta.cache.paimon.partition_view.max-weigth",
"16MB")));
+ Assertions.assertEquals(
+ "Unknown external meta cache property:
meta.cache.paimon.partition_view.max-weigth",
+ misspelled.getMessage());
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () -> validate(props(
+ "paimon.catalog.type", "filesystem", "warehouse",
"/wh",
+ "meta.cache.paimon.partition_view.enable", "1")));
+ }
+
@Test
public void acceptsValidMetaCacheKnobs() {
// Valid values must pass: ttl-second=-1 is the "no expiration"
sentinel (min is -1), 0 disables,
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java
index 00300420aa1..385b42b9bc9 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java
@@ -291,6 +291,8 @@ public class CatalogMgr implements Writable,
GsonPostProcessable {
if (catalog instanceof ExternalCatalog) {
try {
((ExternalCatalog) catalog).checkProperties();
+ ((ExternalCatalog) catalog).checkSubmittedMetaCacheProperties(
+ catalog.getProperties(), cmd.getProperties());
} catch (DdlException | RuntimeException e) {
catalog.onCreateFailure();
throw e;
@@ -447,6 +449,12 @@ public class CatalogMgr implements Writable,
GsonPostProcessable {
}
}
+ if (catalog instanceof ExternalCatalog) {
+ Map<String, String> effectiveProperties =
Maps.newHashMap(oldProperties);
+ effectiveProperties.putAll(updates);
+ ((ExternalCatalog)
catalog).checkSubmittedMetaCacheProperties(effectiveProperties, updates);
+ }
+
// Filesystem binding may read Hadoop XML files. Validate the
detached snapshot before
// taking the global catalog write lock, then retry if another
ALTER changed the snapshot.
boolean validatedWithoutMutation = catalog instanceof
ExternalCatalog
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
index 3934483b97b..2ee07578704 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
@@ -41,6 +41,7 @@ import org.apache.doris.common.util.ExceptionDiagnosticUtils;
import org.apache.doris.common.util.Util;
import org.apache.doris.connector.cache.CacheSpec;
import org.apache.doris.connector.cache.MetaCacheBudgetManager;
+import org.apache.doris.connector.cache.MetaCacheGovernance;
import org.apache.doris.datasource.doris.RemoteDorisExternalDatabase;
import org.apache.doris.datasource.infoschema.ExternalInfoSchemaDatabase;
import org.apache.doris.datasource.infoschema.ExternalMysqlDatabase;
@@ -79,6 +80,7 @@ import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
+import java.util.OptionalLong;
import java.util.Set;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -479,7 +481,34 @@ public abstract class ExternalCatalog
/** Strict CREATE/ALTER validation for the core metadata-cache namespace
owned by this catalog. */
protected void checkMetaCacheWeightProperties(Map<String, String>
properties) {
- CacheSpec.checkWeightProperties(properties, "default", "schema");
+ CacheSpec.checkWeightProperties(properties, coreMetaCacheEngine(),
"schema");
+ }
+
+ /** The engine of the core metadata-cache namespace that fe-core owns for
this catalog. */
+ protected String coreMetaCacheEngine() {
+ return "default";
+ }
+
+ /**
+ * CREATE / ALTER CATALOG only, never on replay: the meta cache keys this
statement submits for the engine of the
+ * core metadata-cache namespace must be well formed, and the weights it
submits must not exceed the FE-global
+ * limit. A connector checks the keys of its own engines when it validates
the statement's properties.
+ */
+ public void checkSubmittedMetaCacheProperties(Map<String, String>
effectiveProperties,
+ Map<String, String> submittedProperties) throws DdlException {
+ checkSubmittedMetaCacheProperties(effectiveProperties,
submittedProperties,
+ MetaCacheGovernance.globalMaxWeight());
+ }
+
+ void checkSubmittedMetaCacheProperties(Map<String, String>
effectiveProperties,
+ Map<String, String> submittedProperties, OptionalLong
globalMaxWeight) throws DdlException {
+ try {
+ CacheSpec.checkSubmittedEngineProperties(submittedProperties,
coreMetaCacheEngine());
+ CacheSpec.checkSubmittedWeightsAgainstGlobal(
+ effectiveProperties, submittedProperties, globalMaxWeight,
coreMetaCacheEngine());
+ } catch (IllegalArgumentException e) {
+ throw new DdlException(e.getMessage());
+ }
}
/**
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/doris/RemoteDorisExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/doris/RemoteDorisExternalCatalog.java
index 0c2397d40f8..2f4bb0c99e6 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/doris/RemoteDorisExternalCatalog.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/doris/RemoteDorisExternalCatalog.java
@@ -81,6 +81,11 @@ public class RemoteDorisExternalCatalog extends
ExternalCatalog {
DorisExternalMetaCache.ENTRY_SCHEMA,
DorisExternalMetaCache.ENTRY_BACKENDS);
}
+ @Override
+ protected String coreMetaCacheEngine() {
+ return DorisExternalMetaCache.ENGINE;
+ }
+
public List<String> getFeNodes() {
return
parseHttpHosts(catalogProperty.getOrDefault(RemoteDorisProperties.FE_HTTP_HOSTS,
""));
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/ExternalCatalogMetaCacheWeightTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/ExternalCatalogMetaCacheWeightTest.java
index 3d8561ab2db..711a73a769d 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/ExternalCatalogMetaCacheWeightTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/ExternalCatalogMetaCacheWeightTest.java
@@ -25,8 +25,10 @@ import org.apache.doris.datasource.test.TestExternalCatalog;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
+import java.util.OptionalLong;
public class ExternalCatalogMetaCacheWeightTest {
@@ -62,6 +64,60 @@ public class ExternalCatalogMetaCacheWeightTest {
});
}
+ @Test
+ public void submittedCoreKeysMustNameAKnownOptionWithAValueOfItsType()
throws DdlException {
+ ExternalCatalog catalog = new TestExternalCatalog(3L, "test", "",
testCatalogProperties(), "");
+ DdlException misspelled = Assertions.assertThrows(DdlException.class,
+ () ->
catalog.checkSubmittedMetaCacheProperties(testCatalogProperties(),
+
Collections.singletonMap("meta.cache.default.schema.max-weigth", "64MB"),
+ OptionalLong.empty()));
+ Assertions.assertTrue(misspelled.getMessage().contains(
+ "Unknown external meta cache property:
meta.cache.default.schema.max-weigth"), misspelled.getMessage());
+ for (String[] bad : new String[][]
{{"meta.cache.default.schema.enable", "1"},
+ {"meta.cache.default.schema.ttl-second", "-2"},
{"meta.cache.default.schema.capacity", "-1"}}) {
+ Assertions.assertThrows(DdlException.class, () ->
catalog.checkSubmittedMetaCacheProperties(
+ testCatalogProperties(), Collections.singletonMap(bad[0],
bad[1]), OptionalLong.empty()), bad[0]);
+ }
+
+ // An entry this version does not know stays accepted; a connector
validates the keys of its own engine.
+ Map<String, String> accepted = new HashMap<>();
+ accepted.put("meta.cache.default.future_entry.max-weight", "64MB");
+ accepted.put("meta.cache.hive.file.enable", "1");
+ catalog.checkSubmittedMetaCacheProperties(testCatalogProperties(),
accepted, OptionalLong.empty());
+ // A key an earlier statement stored is not inspected again.
+ Map<String, String> stored = testCatalogProperties();
+ stored.put("meta.cache.default.schema.max-weigth", "64MB");
+ catalog.checkSubmittedMetaCacheProperties(stored,
Collections.singletonMap("comment", "updated"),
+ OptionalLong.empty());
+ }
+
+ @Test
+ public void submittedWeightsMayNotExceedTheGlobalLimitOfThisFrontend()
throws DdlException {
+ ExternalCatalog catalog = new TestExternalCatalog(4L, "test", "",
testCatalogProperties(), "");
+ OptionalLong global = OptionalLong.of(64L * 1024 * 1024);
+ Map<String, String> catalogAbove =
Collections.singletonMap("meta.cache.max-weight", "128MB");
+ DdlException e = Assertions.assertThrows(DdlException.class,
+ () -> catalog.checkSubmittedMetaCacheProperties(catalogAbove,
catalogAbove, global));
+ Assertions.assertTrue(e.getMessage().contains("meta.cache.max-weight
(134217728 bytes) can not exceed the FE"
+ + " global max weight external_meta_cache_max_weight (67108864
bytes)"), e.getMessage());
+ Map<String, String> entryAbove =
Collections.singletonMap("meta.cache.default.schema.max-weight", "128MB");
+ Assertions.assertThrows(DdlException.class,
+ () -> catalog.checkSubmittedMetaCacheProperties(entryAbove,
entryAbove, global));
+
+ catalog.checkSubmittedMetaCacheProperties(catalogAbove,
Collections.singletonMap("comment", "updated"), global);
+ catalog.checkSubmittedMetaCacheProperties(catalogAbove, catalogAbove,
OptionalLong.empty());
+ }
+
+ @Test
+ public void remoteDorisChecksTheKeysOfItsOwnEngine() throws DdlException {
+ ExternalCatalog catalog = new RemoteDorisExternalCatalog(3L, "remote",
"", remoteDorisProperties(), "");
+ Assertions.assertThrows(DdlException.class, () ->
catalog.checkSubmittedMetaCacheProperties(
+ remoteDorisProperties(),
Collections.singletonMap("meta.cache.doris.backends.enable", "1"),
+ OptionalLong.empty()));
+ catalog.checkSubmittedMetaCacheProperties(remoteDorisProperties(),
+
Collections.singletonMap("meta.cache.doris.future_entry.ttl-second", "0"),
OptionalLong.empty());
+ }
+
private static Map<String, String> testCatalogProperties() {
Map<String, String> properties = new HashMap<>();
properties.put("catalog_provider.class",
RefreshCatalogTest.RefreshCatalogProvider.class.getName());
diff --git a/regression-test/suites/external_table_p0/test_catalog_ddl.groovy
b/regression-test/suites/external_table_p0/test_catalog_ddl.groovy
index a75159a00f1..07b9a84ba49 100644
--- a/regression-test/suites/external_table_p0/test_catalog_ddl.groovy
+++ b/regression-test/suites/external_table_p0/test_catalog_ddl.groovy
@@ -153,4 +153,58 @@ suite("test_catalog_ddl", "p0,external") {
)
"""
sql """alter catalog test_ddl_future_core_cache set
properties("comment" = "updated")"""
+
+ // A submitted meta cache key must name a known option with a value of
its type. An unknown entry is still
+ // accepted (above): a newer connector may read an entry this version
does not know.
+ sql """drop catalog if exists test_ddl_strict_meta_cache"""
+ test {
+ sql """
+ create catalog test_ddl_strict_meta_cache properties(
+ "type" = "hms",
+ "hive.metastore.uris" = "thrift://127.0.0.1:9083",
+ "meta.cache.hive.file.max-weigth" = "64MB"
+ )
+ """
+ exception "Unknown external meta cache property:
meta.cache.hive.file.max-weigth"
+ }
+ test {
+ sql """
+ create catalog test_ddl_strict_meta_cache properties(
+ "type" = "hms",
+ "hive.metastore.uris" = "thrift://127.0.0.1:9083",
+ "meta.cache.hive.file.enable" = "1"
+ )
+ """
+ exception "The parameter meta.cache.hive.file.enable is wrong,
value is 1"
+ }
+ test {
+ sql """
+ create catalog test_ddl_strict_meta_cache properties(
+ "type" = "hms",
+ "hive.metastore.uris" = "thrift://127.0.0.1:9083",
+ "meta.cache.iceberg.table.ttl-second" = "-2"
+ )
+ """
+ exception "The parameter meta.cache.iceberg.table.ttl-second is
wrong, value is -2"
+ }
+ // The core namespace that fe-core owns is checked the same way, on
ALTER too.
+ test {
+ sql """alter catalog test_ddl_future_core_cache set
properties("meta.cache.default.schema.enable" = "1")"""
+ exception "The parameter meta.cache.default.schema.enable is
wrong, value is 1"
+ }
+
+ // 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") {
+ test {
+ sql """
+ create catalog test_ddl_strict_meta_cache properties(
+ "type" = "hms",
+ "hive.metastore.uris" = "thrift://127.0.0.1:9083",
+ "meta.cache.max-weight" = "1PB"
+ )
+ """
+ exception "can not exceed the FE global max weight
external_meta_cache_max_weight"
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
