github-actions[bot] commented on code in PR #66717:
URL: https://github.com/apache/doris/pull/66717#discussion_r3796838722


##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonCacheSizeEstimator.java:
##########
@@ -0,0 +1,448 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.datasource.paimon;
+
+import org.apache.doris.datasource.metacache.MetaCacheSizeEstimate;
+import org.apache.doris.datasource.metacache.MetaCacheWeightUtils;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.paimon.privilege.PrivilegedFileStoreTable;
+import org.apache.paimon.schema.TableSchema;
+import org.apache.paimon.table.FallbackReadFileStoreTable;
+import org.apache.paimon.table.FileStoreTable;
+import org.apache.paimon.table.Table;
+import org.apache.paimon.types.ArrayType;
+import org.apache.paimon.types.BigIntType;
+import org.apache.paimon.types.BinaryType;
+import org.apache.paimon.types.BlobType;
+import org.apache.paimon.types.BooleanType;
+import org.apache.paimon.types.CharType;
+import org.apache.paimon.types.DataField;
+import org.apache.paimon.types.DataType;
+import org.apache.paimon.types.DateType;
+import org.apache.paimon.types.DecimalType;
+import org.apache.paimon.types.DoubleType;
+import org.apache.paimon.types.FloatType;
+import org.apache.paimon.types.IntType;
+import org.apache.paimon.types.LocalZonedTimestampType;
+import org.apache.paimon.types.MapType;
+import org.apache.paimon.types.MultisetType;
+import org.apache.paimon.types.RowType;
+import org.apache.paimon.types.SmallIntType;
+import org.apache.paimon.types.TimeType;
+import org.apache.paimon.types.TimestampType;
+import org.apache.paimon.types.TinyIntType;
+import org.apache.paimon.types.VarBinaryType;
+import org.apache.paimon.types.VarCharType;
+import org.apache.paimon.types.VariantType;
+import org.apache.paimon.types.VectorType;
+
+import java.util.List;
+import java.util.Map;
+
+/** Publication-time retained-weight formula for Paimon snapshot projections. 
*/
+final class PaimonCacheSizeEstimator {
+    // Calibrated against JOL retained-graph deltas in 
PaimonExternalMetaCacheTest.
+    private static final long MAX_TABLE_ACCOUNTING_ELEMENTS = 50_000L;
+    private static final int MAX_TYPE_ACCOUNTING_DEPTH = 128;
+    private static final long KEY_BASE_BYTES = objectBytes(128L);
+    private static final long SNAPSHOT_BASE_BYTES = objectBytes(4L * 1024L);
+    private static final long TABLE_BASE_BYTES = objectBytes(16L * 1024L);
+    // A top-level DataField, its list slot and shared per-field overhead; the 
DataType instance
+    // is accounted separately by addTypePayload.
+    private static final long TABLE_FIELD_BYTES = objectBytes(40L);
+    private static final long TABLE_OPTION_BYTES = objectBytes(44L);
+    private static final long TABLE_KEY_BYTES = objectBytes(128L);
+    // Exact Paimon 1.4.2 layouts, pinned by PAIMON_TYPE_LAYOUT_SUPPORTED.
+    private static final long DATA_FIELD_BYTES =
+            MetaCacheWeightUtils.estimatedObjectLayoutBytes(4L, 4L);
+    private static final long ARRAY_TYPE_BYTES =
+            MetaCacheWeightUtils.estimatedObjectLayoutBytes(2L, 1L);
+    private static final long VECTOR_TYPE_BYTES =
+            MetaCacheWeightUtils.estimatedObjectLayoutBytes(2L, 5L);
+    private static final long MAP_TYPE_BYTES =
+            MetaCacheWeightUtils.estimatedObjectLayoutBytes(3L, 1L);
+    private static final long MULTISET_TYPE_BYTES =
+            MetaCacheWeightUtils.estimatedObjectLayoutBytes(2L, 1L);
+    // RowType plus Collections.unmodifiableList(new ArrayList<>(fields)).
+    private static final long ROW_TYPE_BYTES =
+            MetaCacheWeightUtils.estimatedObjectLayoutBytes(6L, 1L);
+    private static final long UNMODIFIABLE_LIST_BYTES =
+            MetaCacheWeightUtils.estimatedObjectLayoutBytes(2L, 0L);
+    private static final long ARRAY_LIST_BYTES =
+            MetaCacheWeightUtils.estimatedObjectLayoutBytes(1L, 8L);
+    private static final long HASH_MAP_BYTES =
+            MetaCacheWeightUtils.estimatedObjectLayoutBytes(4L, 16L);
+    private static final long HASH_MAP_NODE_BYTES =
+            MetaCacheWeightUtils.estimatedObjectLayoutBytes(3L, 4L);
+    private static final long INTEGER_BYTES =
+            MetaCacheWeightUtils.estimatedObjectLayoutBytes(0L, 4L);
+    private static final int ROW_TYPE_LAZY_MAP_COUNT = 4;
+    // Accepted leaf DataType implementations and the int fields each adds to 
DataType's nullable
+    // flag and type root. Any other class, including a future or third-party 
type, rejects
+    // weighted admission instead of being counted as an arbitrary primitive.
+    private static final String[] NO_LEAF_FIELDS = {};
+    private static final String[] LENGTH_LEAF_FIELDS = {"length:int"};
+    private static final String[] PRECISION_LEAF_FIELDS = {"precision:int"};
+    private static final Map<Class<? extends DataType>, String[]> 
LEAF_TYPE_FIELDS =
+            ImmutableMap.<Class<? extends DataType>, String[]>builder()
+                    .put(CharType.class, LENGTH_LEAF_FIELDS)
+                    .put(VarCharType.class, LENGTH_LEAF_FIELDS)
+                    .put(BooleanType.class, NO_LEAF_FIELDS)
+                    .put(BinaryType.class, LENGTH_LEAF_FIELDS)
+                    .put(VarBinaryType.class, LENGTH_LEAF_FIELDS)
+                    .put(DecimalType.class, new String[] {"precision:int", 
"scale:int"})
+                    .put(TinyIntType.class, NO_LEAF_FIELDS)
+                    .put(SmallIntType.class, NO_LEAF_FIELDS)
+                    .put(IntType.class, NO_LEAF_FIELDS)
+                    .put(BigIntType.class, NO_LEAF_FIELDS)
+                    .put(FloatType.class, NO_LEAF_FIELDS)
+                    .put(DoubleType.class, NO_LEAF_FIELDS)
+                    .put(DateType.class, NO_LEAF_FIELDS)
+                    .put(TimeType.class, PRECISION_LEAF_FIELDS)
+                    .put(TimestampType.class, PRECISION_LEAF_FIELDS)
+                    .put(LocalZonedTimestampType.class, PRECISION_LEAF_FIELDS)
+                    .put(VariantType.class, NO_LEAF_FIELDS)
+                    .put(BlobType.class, NO_LEAF_FIELDS)
+                    .build();
+    private static final boolean PAIMON_TYPE_LAYOUT_SUPPORTED = 
checkPaimonTypeLayout();
+    private static final boolean PAIMON_TABLE_LAYOUT_SUPPORTED = 
checkPaimonTableLayout();
+    private static final long PARTITION_BYTES = objectBytes(160L);
+    private static final long PARTITION_ITEM_BYTES = objectBytes(640L);
+    private static final long WRAPPER_BYTES = objectBytes(512L);
+
+    private PaimonCacheSizeEstimator() {
+    }
+
+    private static long objectBytes(long bytes) {
+        return MetaCacheWeightUtils.estimatedObjectBytes(bytes);
+    }
+
+    /** DataType: typeRoot reference plus the isNullable flag, then the 
subclass int fields. */
+    private static long leafTypeBytes(String[] intFields) {
+        return MetaCacheWeightUtils.estimatedObjectLayoutBytes(
+                1L, 1L + (long) Integer.BYTES * intFields.length);
+    }
+
+    /** Pin the Paimon 1.4.2 DataType/DataField/RowType layouts the formulas 
above are built on. */
+    private static boolean checkPaimonTypeLayout() {
+        boolean supported = MetaCacheWeightUtils.hasExpectedInstanceFields(
+                DataType.class, "isNullable:boolean", "typeRoot:DataTypeRoot")
+                && MetaCacheWeightUtils.hasExpectedInstanceFields(
+                        DataField.class, "id:int", "name:String", 
"type:DataType",
+                        "description:String", "defaultValue:String")
+                && MetaCacheWeightUtils.hasExpectedInstanceFields(
+                        RowType.class, "fields:List", "laziedNameToField:Map",
+                        "laziedNameToIndex:Map", "laziedFieldIdToField:Map",
+                        "laziedFieldIdToIndex:Map")
+                && MetaCacheWeightUtils.hasExpectedInstanceFields(
+                        ArrayType.class, "elementType:DataType")
+                && MetaCacheWeightUtils.hasExpectedInstanceFields(
+                        VectorType.class, "elementType:DataType", "length:int")
+                && MetaCacheWeightUtils.hasExpectedInstanceFields(
+                        MapType.class, "keyType:DataType", 
"valueType:DataType")
+                && MetaCacheWeightUtils.hasExpectedInstanceFields(
+                        MultisetType.class, "elementType:DataType");
+        for (Map.Entry<Class<? extends DataType>, String[]> leaf : 
LEAF_TYPE_FIELDS.entrySet()) {
+            supported &= MetaCacheWeightUtils.hasExpectedInstanceFields(
+                    leaf.getKey(), leaf.getValue());
+        }
+        return supported;
+    }
+
+    /** Pin TableSchema and the two accepted FileStoreTable implementations. */
+    private static boolean checkPaimonTableLayout() {
+        ClassLoader loader = FileStoreTable.class.getClassLoader();
+        String[] abstractTableFields = {
+                "fileIO:FileIO", "path:Path", "tableSchema:TableSchema",
+                "catalogEnvironment:CatalogEnvironment", 
"manifestCache:SegmentsCache",
+                "snapshotCache:Cache", "statsCache:Cache", 
"dvmetaCache:DVMetaCache"};
+        return MetaCacheWeightUtils.hasExpectedInstanceFields(
+                TableSchema.class, "version:int", "id:long", "fields:List",
+                "highestFieldId:int", "partitionKeys:List", "primaryKeys:List",
+                "bucketKeys:List", "numBucket:int", "options:Map", 
"comment:String",
+                "timeMillis:long")
+                && MetaCacheWeightUtils.hasExpectedInstanceFields(
+                        "org.apache.paimon.table.AbstractFileStoreTable", 
loader,
+                        abstractTableFields)
+                && MetaCacheWeightUtils.hasExpectedInstanceFields(
+                        "org.apache.paimon.table.AppendOnlyFileStoreTable", 
loader,
+                        "lazyStore:AppendOnlyFileStore")
+                && MetaCacheWeightUtils.hasExpectedInstanceFields(
+                        "org.apache.paimon.table.PrimaryKeyFileStoreTable", 
loader,
+                        "lazyStore:KeyValueFileStore");
+    }
+
+    static MetaCacheSizeEstimate estimateSnapshotEntry(
+            PaimonSnapshotEntryKey key, PaimonSnapshotCacheValue value) {
+        if (!MetaCacheWeightUtils.isSupportedJvmObjectLayout()) {
+            return 
MetaCacheSizeEstimate.incomplete("unsupported_jvm_object_alignment");
+        }
+        if (!PAIMON_TYPE_LAYOUT_SUPPORTED || !PAIMON_TABLE_LAYOUT_SUPPORTED) {
+            return 
MetaCacheSizeEstimate.incomplete("unsupported_paimon_layout");
+        }
+        Table table = value.getSnapshot().getTable();
+        if (!isSupportedTable(table)) {
+            return MetaCacheSizeEstimate.incomplete("unsupported_paimon_table:"
+                    + (table == null ? "null" : table.getClass().getName()));
+        }
+
+        long bytes = MetaCacheWeightUtils.saturatedAdd(
+                KEY_BASE_BYTES, 
MetaCacheWeightUtils.estimatedNameMappingBytes(key.getNameMapping()));
+        bytes = MetaCacheWeightUtils.saturatedAdd(bytes, SNAPSHOT_BASE_BYTES);
+        bytes = addCount(bytes, 
value.getPartitionInfo().getNameToPartition().size(), PARTITION_BYTES);
+        bytes = addCount(bytes, 
value.getPartitionInfo().getNameToPartitionItem().size(), PARTITION_ITEM_BYTES);

Review Comment:
   [P2] Account for partition width in the Paimon weight
   
   This adds a fixed `PARTITION_ITEM_BYTES` per partition, but 
`toListPartitionItem()` retains one `LiteralExpr` plus entries/backing-array 
growth in `PartitionKey.keys`, `originHiveKeys`, and `types` for every 
partition column. `retainedPayloadBytes` charges the strings, not those 
structural objects, so the real graph grows with `partitionCount * 
partitionColumnCount` while this formula does not. Please carry a per-column 
structural count and add a narrow-vs-wide JOL calibration; the current fixture 
has only one column.



##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -446,6 +447,19 @@ protected void checkProperties(CatalogProperty property) 
throws DdlException {
             }
         }
 
+        try {
+            Env currentEnv = Env.getCurrentEnv();
+            ExternalMetaCacheMgr extMetaCacheMgr = currentEnv == null ? null : 
currentEnv.getExtMetaCacheMgr();
+            if (extMetaCacheMgr == null) {
+                // This fallback is only for isolated construction tests 
before Env is initialized.
+                
ExternalMetaCacheBudgetManager.fromConfig().validateCatalogMaxWeight(properties);
+            } else {
+                extMetaCacheMgr.validateCatalogCacheProperties(this, 
properties);

Review Comment:
   [P2] Do not lock legacy catalogs out of ALTER
   
   Image/replay deliberately admits legacy cache options and lazy 
initialization sanitizes them, but this call validates the entire persisted map 
on every later ALTER. An old unknown or obsolete cache key therefore rejects 
even an unrelated valid update, and `ALTER CATALOG` only supports SET/`putAll`, 
so the user cannot remove it. Please validate newly supplied cache keys plus 
any affected hierarchy, or provide a migration/removal path, and cover an old 
typo followed by an unrelated ALTER.



##########
fe/fe-core/src/main/java/org/apache/doris/datasource/metacache/MetaCacheEntryStats.java:
##########
@@ -51,6 +51,16 @@ public final class MetaCacheEntryStats {
     private final long lastLoadSuccessTimeMs;
     private final long lastLoadFailureTimeMs;
     private final String lastError;
+    private final boolean weightBounded;
+    private final long maxWeight;
+    private final long estimatedWeight;

Review Comment:
   [P2] Expose the new weight statistics at the public boundary
   
   These fields are populated by `MetaCacheEntry.stats()`, but their getters 
have no production consumer: `information_schema.catalog_meta_cache_statistics` 
and `MetadataGenerator.metaCacheStatsMetadataResult()` still stop at the old 
`LAST_ERROR` column. The rejection WARN is throttled and reports only 
rejection-time state, so the supported statistics view still cannot show 
current hierarchy usage, eviction weight, rejection count, or the last 
rejection reason. Please extend that table/generator, or wire these fields into 
an equivalent documented metrics surface.



-- 
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]

Reply via email to