github-actions[bot] commented on code in PR #66717:
URL: https://github.com/apache/doris/pull/66717#discussion_r3774111186
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergSnapshotCacheValue.java:
##########
@@ -221,4 +316,30 @@ private boolean isWriterCompatible(TableMetadata
refreshedMetadata) {
&& Objects.equals(retainedMetadata.properties(),
refreshedMetadata.properties());
}
}
+
+ private static class ServiceBackedTableOperations extends
RetainedTableOperations {
+ private TableMetadata currentMetadata;
+
+ private ServiceBackedTableOperations(TableOperations delegate,
TableMetadata retainedMetadata) {
+ super(delegate, retainedMetadata);
+ this.currentMetadata = retainedMetadata;
+ }
+
+ @Override
+ public TableMetadata current() {
+ return currentMetadata;
+ }
+
+ @Override
+ public TableMetadata refresh() {
+ currentMetadata = delegate.refresh();
+ return currentMetadata;
+ }
+
+ @Override
+ public void commit(TableMetadata base, TableMetadata newMetadata) {
+ delegate.commit(base, newMetadata);
Review Comment:
[P1] Rebind detached metadata updates before delegating the commit
When this table entry is weight-managed, publication JSON-detaches its
`TableMetadata`, and `getIcebergTable()` seeds `ServiceBackedTableOperations`
with another detached object. Iceberg 1.10.1 `SchemaUpdate` and
`BaseUpdatePartitionSpec` capture `ops.current()` and call `ops.commit(base,
update)` without refreshing, while `HadoopTableOperations` and
`BaseMetastoreTableOperations` require `base` to be their current object by
identity. Forwarding this clone therefore makes ALTER TABLE schema/reorder and
partition evolution fail as stale for Hadoop, Hive, JDBC, Glue, and DLF
catalogs whenever a table, catalog, or global weight limit enables this path.
Please rebind a verified retained generation to the delegate's actual current
object and cover both update kinds under weighted caching.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergTableCacheValue.java:
##########
@@ -17,25 +17,41 @@
package org.apache.doris.datasource.iceberg;
-import com.google.common.base.Suppliers;
-import org.apache.iceberg.Table;
+import org.apache.doris.datasource.NameMapping;
+import org.apache.doris.datasource.metacache.MetaCacheSizeEstimate;
+import org.apache.doris.datasource.metacache.MetaCacheSizeEstimator;
-import java.util.function.Supplier;
+import org.apache.iceberg.Table;
public class IcebergTableCacheValue {
- private final Table icebergTable;
- private final Supplier<IcebergSnapshotCacheValue> latestSnapshotCacheValue;
+ private Table icebergTable;
+ private MetaCacheSizeEstimate sizeEstimate;
- public IcebergTableCacheValue(Table icebergTable,
Supplier<IcebergSnapshotCacheValue> latestSnapshotCacheValue) {
+ public IcebergTableCacheValue(Table icebergTable) {
this.icebergTable = icebergTable;
- this.latestSnapshotCacheValue =
Suppliers.memoize(latestSnapshotCacheValue::get);
}
public Table getIcebergTable() {
- return icebergTable;
+ return
IcebergSnapshotCacheValue.createServiceBackedTable(icebergTable);
+ }
+
+ MetaCacheSizeEstimate prepareForCachePublication(NameMapping key) {
+ if (sizeEstimate == null) {
+ sizeEstimate =
MetaCacheSizeEstimator.estimateSafely("iceberg_table_preparation_failed", () ->
{
+ icebergTable =
IcebergSnapshotCacheValue.detachTableGeneration(icebergTable);
+
IcebergSnapshotCacheValue.materializeAllSnapshotManifests(icebergTable);
Review Comment:
[P2] Avoid materializing all historical manifests for an unexposed graph
Weighted table admission now calls both manifest accessors for every
historical snapshot. For v2 snapshots this lazily reads one manifest-list file
per snapshot. The lists are retained only in this hidden frozen table for
weighing: every public `getIcebergTable()` JSON-clones the metadata into a new
service-backed table, dropping Iceberg's transient memoized lists, so scans
cannot reuse the work. Long-lived tables therefore pay history-linear work and
v2 remote I/O on each admission/refresh solely for an unused graph. Please
weigh only metadata that callers can reuse, and add a many-snapshot v2
FileIO-read regression.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/metacache/AbstractExternalMetaCache.java:
##########
@@ -81,10 +89,29 @@ public Collection<String> aliases() {
}
@Override
- public void initCatalog(long catalogId, Map<String, String>
catalogProperties) {
+ public void validateCatalogProperties(Map<String, String>
catalogProperties) {
Map<String, String> safeCatalogProperties =
CacheSpec.applyCompatibilityMap(
catalogProperties, catalogPropertyCompatibilityMap());
- catalogEntries.computeIfAbsent(catalogId, id ->
buildCatalogEntryGroup(safeCatalogProperties));
+ validateMappedCatalogProperties(safeCatalogProperties);
+ }
+
+ @Override
+ public synchronized void initCatalog(long catalogId, Map<String, String>
catalogProperties) {
Review Comment:
[P2] Keep initialized cache lookups off the engine-wide monitor
Every `ExternalMetaCacheMgr` typed accessor unconditionally calls
`prepareCatalogByEngine`, which copies and validates the properties, and then
reaches this synchronized method. Even when the catalog group already exists,
the lookup therefore serializes with every other catalog using this engine and
repeats compatibility mapping plus hierarchy validation before
`computeIfAbsent` discovers there is no work. This is on normal planning paths
such as Iceberg table and Paimon snapshot/schema lookup, so parallel queries
across unrelated catalogs acquire one global engine lock. Please add a
lock-free initialized fast path and reserve synchronization/validation for the
first build after create or invalidation.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonSnapshotEntryKey.java:
##########
@@ -0,0 +1,70 @@
+// 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.NameMapping;
+
+import java.util.Objects;
+
+/** Stable identity for a Paimon projection hydrated from one captured
snapshot/schema fence. */
+public final class PaimonSnapshotEntryKey {
+ private final NameMapping nameMapping;
+ private final long snapshotId;
+ private final long schemaId;
+
+ public PaimonSnapshotEntryKey(NameMapping nameMapping, long snapshotId,
long schemaId) {
+ this.nameMapping = Objects.requireNonNull(nameMapping, "nameMapping
can not be null");
+ this.snapshotId = snapshotId;
+ this.schemaId = schemaId;
+ }
+
+ public static PaimonSnapshotEntryKey of(NameMapping nameMapping,
PaimonSnapshot fence) {
+ return new PaimonSnapshotEntryKey(nameMapping, fence.getSnapshotId(),
fence.getSchemaId());
+ }
+
+ public NameMapping getNameMapping() {
+ return nameMapping;
+ }
+
+ public long getSnapshotId() {
+ return snapshotId;
+ }
+
+ public long getSchemaId() {
+ return schemaId;
+ }
+
+ @Override
+ public boolean equals(Object object) {
+ if (this == object) {
+ return true;
+ }
+ if (!(object instanceof PaimonSnapshotEntryKey)) {
+ return false;
+ }
+ PaimonSnapshotEntryKey that = (PaimonSnapshotEntryKey) object;
+ return snapshotId == that.snapshotId
Review Comment:
[P1] Include the table generation in the snapshot-cache identity
The contextual value retains the fenced Paimon `Table` and its partition
projection, but equality uses only the table name plus snapshot/schema IDs. The
table entry refreshes independently, while this contextual entry cannot
auto-refresh and has its own TTL. After a drop/recreate (where IDs restart) or
another same-ID physical table generation, `getSnapshotCache()` can read the
new table fence and still hit the old value, returning the old table handle and
partition map. Explicit invalidation clears both entries, but ordinary table
refresh/replacement does not. Please add a stable table-generation/options
identity to this key or couple every table-entry replacement to snapshot
invalidation, with a same-ID replacement regression test.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergSnapshotCacheValue.java:
##########
@@ -119,6 +202,18 @@ static Table createWritableTable(Table retainedTable,
Table liveTable) {
new WritableTableOperations(liveOperations, retainedMetadata));
}
+ static Table createServiceBackedTable(Table retainedTable) {
+ if (!isFrozenGeneration(retainedTable)) {
+ return retainedTable;
+ }
+ TableOperations retainedOperations = ((HasTableOperations)
retainedTable).operations();
+ TableMetadata retainedMetadata = retainedOperations.current();
+ TableMetadata callerMetadata = TableMetadataParser.fromJson(
Review Comment:
[P2] Avoid rebuilding full metadata on every weighted cache hit
Once weighted publication freezes this table entry, every
`getIcebergTable()` reaches these calls and serializes then reparses the
complete `TableMetadata`: schemas, specs, sort orders, snapshots, refs,
statistics, and history logs. This runs on ordinary table/scan planning and
even before `getSnapshotCache()` can hit its contextual entry, so a successful
cache hit still performs history-linear CPU and allocation and builds a
transient graph outside the retained budget. Count-only entries take the early
return. Please reuse a safe read-only frozen projection and construct an
isolated service-backed/writable wrapper only for mutation paths, with a
long-history cache-hit benchmark.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/metacache/OwnedObjectSizeEstimator.java:
##########
@@ -0,0 +1,712 @@
+// 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.metacache;
+
+import org.apache.doris.analysis.LiteralExpr;
+import org.apache.doris.analysis.MaxLiteral;
+import org.apache.doris.catalog.ListPartitionItem;
+import org.apache.doris.catalog.PartitionKey;
+import org.apache.doris.catalog.RangePartitionItem;
+import org.apache.doris.catalog.ScalarType;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.nereids.trees.expressions.literal.Literal;
+
+import java.lang.reflect.Array;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.nio.ByteBuffer;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.BitSet;
+import java.util.Collection;
+import java.util.Deque;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Full, identity-deduplicated traversal for owned Doris cache value graphs.
+ *
+ * <p>JDK containers use public collection APIs and explicit layout formulas,
so
+ * this walker never needs module opens. An inaccessible non-JDK owned field
makes
+ * the result incomplete instead of silently contributing zero. Unknown
mutable ArrayLists
+ * are rejected because their retained capacity is not exposed by a public
API. The dedicated
+ * Hive partition adapters conservatively model lists created by the
connector's known path;
+ * cache publishers freeze all other owned lists before estimation.
+ */
+public final class OwnedObjectSizeEstimator {
+ private static final long LEGACY_LITERAL_HEADROOM_BYTES = 1024L;
+ private static final long NEREIDS_LITERAL_HEADROOM_BYTES = 2048L;
+ // These are hard safety ceilings, not sampling limits. Production-sized
Hive partition graphs
+ // are still traversed exactly; an object is rejected if exact traversal
cannot finish safely.
+ static final int DEFAULT_MAX_OBJECTS = 5_000_000;
+ static final int DEFAULT_MAX_DEPTH = 128;
+ static final long DEFAULT_TIMEOUT_NANOS = TimeUnit.SECONDS.toNanos(5);
+ private static final int DEADLINE_CHECK_INTERVAL = 256;
+ private static final int CIRCUIT_FAILURE_THRESHOLD = 3;
+ private static final long CIRCUIT_OPEN_NANOS = TimeUnit.MINUTES.toNanos(1);
+ private static final Map<Class<?>, FailureCircuit> FAILURE_CIRCUITS = new
ConcurrentHashMap<>();
+ private static final Set<String> SUPPORTED_LEGACY_LITERAL_TYPES = new
HashSet<>(Arrays.asList(
+ "org.apache.doris.analysis.BoolLiteral",
+ "org.apache.doris.analysis.DateLiteral",
+ "org.apache.doris.analysis.DecimalLiteral",
+ "org.apache.doris.analysis.FloatLiteral",
+ "org.apache.doris.analysis.IPv4Literal",
+ "org.apache.doris.analysis.IPv6Literal",
+ "org.apache.doris.analysis.IntLiteral",
+ "org.apache.doris.analysis.JsonLiteral",
+ "org.apache.doris.analysis.LargeIntLiteral",
+ "org.apache.doris.analysis.MaxLiteral",
+ "org.apache.doris.analysis.NullLiteral",
+ "org.apache.doris.analysis.StringLiteral",
+ "org.apache.doris.analysis.TimeV2Literal",
+ "org.apache.doris.analysis.VarBinaryLiteral"));
+ private static final Set<String> SUPPORTED_NEREIDS_LITERAL_TYPES = new
HashSet<>(Arrays.asList(
+ "org.apache.doris.nereids.trees.expressions.literal.BigIntLiteral",
+
"org.apache.doris.nereids.trees.expressions.literal.BooleanLiteral",
+ "org.apache.doris.nereids.trees.expressions.literal.CharLiteral",
+ "org.apache.doris.nereids.trees.expressions.literal.DateLiteral",
+
"org.apache.doris.nereids.trees.expressions.literal.DateTimeLiteral",
+
"org.apache.doris.nereids.trees.expressions.literal.DateTimeV2Literal",
+ "org.apache.doris.nereids.trees.expressions.literal.DateV2Literal",
+
"org.apache.doris.nereids.trees.expressions.literal.DecimalLiteral",
+
"org.apache.doris.nereids.trees.expressions.literal.DecimalV3Literal",
+ "org.apache.doris.nereids.trees.expressions.literal.DoubleLiteral",
+ "org.apache.doris.nereids.trees.expressions.literal.FloatLiteral",
+ "org.apache.doris.nereids.trees.expressions.literal.IPv4Literal",
+ "org.apache.doris.nereids.trees.expressions.literal.IPv6Literal",
+
"org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral",
+ "org.apache.doris.nereids.trees.expressions.literal.JsonLiteral",
+
"org.apache.doris.nereids.trees.expressions.literal.LargeIntLiteral",
+ "org.apache.doris.nereids.trees.expressions.literal.MaxLiteral",
+ "org.apache.doris.nereids.trees.expressions.literal.NullLiteral",
+
"org.apache.doris.nereids.trees.expressions.literal.SmallIntLiteral",
+ "org.apache.doris.nereids.trees.expressions.literal.StringLiteral",
+ "org.apache.doris.nereids.trees.expressions.literal.TimeV2Literal",
+
"org.apache.doris.nereids.trees.expressions.literal.TimestampTzLiteral",
+
"org.apache.doris.nereids.trees.expressions.literal.TinyIntLiteral",
+
"org.apache.doris.nereids.trees.expressions.literal.VarBinaryLiteral",
+
"org.apache.doris.nereids.trees.expressions.literal.VarcharLiteral"));
+ private static final Set<String> SHARED_BOUNDARY_TYPE_NAMES = new
LinkedHashSet<>();
+
+ static {
+ SHARED_BOUNDARY_TYPE_NAMES.add("java.util.concurrent.Executor");
+ SHARED_BOUNDARY_TYPE_NAMES.add("java.util.concurrent.ExecutorService");
+ SHARED_BOUNDARY_TYPE_NAMES.add("javax.sql.DataSource");
+
SHARED_BOUNDARY_TYPE_NAMES.add("org.apache.doris.common.security.authentication.HadoopAuthenticator");
+
SHARED_BOUNDARY_TYPE_NAMES.add("org.apache.doris.datasource.CatalogIf");
+ SHARED_BOUNDARY_TYPE_NAMES.add("org.apache.hadoop.conf.Configuration");
+ SHARED_BOUNDARY_TYPE_NAMES.add("org.apache.hadoop.fs.FileSystem");
+
SHARED_BOUNDARY_TYPE_NAMES.add("org.apache.hadoop.hive.metastore.IMetaStoreClient");
+ SHARED_BOUNDARY_TYPE_NAMES.add("org.apache.iceberg.catalog.Catalog");
+ SHARED_BOUNDARY_TYPE_NAMES.add("org.apache.iceberg.io.FileIO");
+ SHARED_BOUNDARY_TYPE_NAMES.add("org.apache.iceberg.Table");
+ SHARED_BOUNDARY_TYPE_NAMES.add("org.apache.paimon.catalog.Catalog");
+ SHARED_BOUNDARY_TYPE_NAMES.add("org.apache.paimon.fs.FileIO");
+ SHARED_BOUNDARY_TYPE_NAMES.add("org.apache.paimon.table.Table");
+ }
+
+ private static final ClassValue<Boolean> SHARED_BOUNDARY_TYPES = new
ClassValue<Boolean>() {
+ @Override
+ protected Boolean computeValue(Class<?> type) {
+ return isNamedBoundaryType(type);
+ }
+ };
+
+ private static final ClassValue<ClassPlan> CLASS_PLANS = new
ClassValue<ClassPlan>() {
+ @Override
+ protected ClassPlan computeValue(Class<?> type) {
+ List<Field> referenceFields = new ArrayList<>();
+ for (Class<?> current = type; current != null; current =
current.getSuperclass()) {
+ for (Field field : current.getDeclaredFields()) {
+ if (Modifier.isStatic(field.getModifiers()) ||
field.getType().isPrimitive()) {
+ continue;
+ }
+ try {
+ if (!field.isAccessible()) {
+ field.setAccessible(true);
+ }
+ referenceFields.add(field);
+ } catch (RuntimeException e) {
+ return ClassPlan.inaccessible(current.getName() + "."
+ field.getName());
+ }
+ }
+ }
+ return ClassPlan.accessible(referenceFields.toArray(new Field[0]));
+ }
+ };
+
+ private OwnedObjectSizeEstimator() {
+ }
+
+ public static MetaCacheSizeEstimate estimate(Object... roots) {
+ Set<Class<?>> rootTypes = new LinkedHashSet<>();
+ for (Object root : roots) {
+ if (root != null) {
+ rootTypes.add(root.getClass());
+ }
+ }
+ long now = System.nanoTime();
+ for (Class<?> rootType : rootTypes) {
+ FailureCircuit circuit = FAILURE_CIRCUITS.get(rootType);
Review Comment:
[P2] Do not circuit-break data-dependent failures by root class
This circuit is static and keyed only by the root Java class. After three
equal incomplete results, every graph with that class is rejected for a minute
before inspection. Reasons such as `object_budget_exceeded` and
`time_budget_exceeded` are graph-dependent, so three large
`HivePartitionValues` entries can make small, fully supported tables in
unrelated catalogs miss the weighted cache as `estimator_circuit_open`; a valid
sibling cannot record success because it is short-circuited here. Please scope
suppression to the actual cache owner/key or only class-break deterministic
unsupported-type failures, and test an invalid root followed by a valid root of
the same class.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergPartition.java:
##########
@@ -79,4 +81,10 @@ public List<String> getPartitionValues() {
public List<String> getTransforms() {
return transforms;
}
+
+ IcebergPartition immutableCopy() {
+ return new IcebergPartition(partitionName, specId, recordCount,
fileSizeInBytes, fileCount,
+ lastUpdateTime, lastSnapshotId,
ImmutableList.copyOf(partitionValues),
Review Comment:
[P2] Preserve supported NULL partition values in the immutable copy
`generateIcebergPartition()` intentionally appends Java `null` for a NULL
partition field, and `getPartitionRange()` has a dedicated null branch, but
Guava `ImmutableList.copyOf` rejects null elements here. With direct or
inherited snapshot weight governance, publication therefore produces an
incomplete estimate and serves the projection only once without caching it;
every later lookup re-enumerates the partitions metadata table. Please use an
ownership-isolated, null-tolerant unmodifiable copy and add a weighted
snapshot-cache test with a NULL partition value.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonCacheSizeEstimator.java:
##########
@@ -0,0 +1,271 @@
+// 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.JvmSizeUtils;
+import org.apache.doris.datasource.metacache.MetaCacheSizeEstimate;
+import org.apache.doris.datasource.metacache.OwnedObjectSizeEstimator;
+
+import org.apache.paimon.FileStore;
+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.DataField;
+import org.apache.paimon.types.DataType;
+import org.apache.paimon.types.MapType;
+import org.apache.paimon.types.MultisetType;
+import org.apache.paimon.types.RowType;
+import org.apache.paimon.types.VectorType;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.Collections;
+import java.util.IdentityHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/** Full construction-time estimator for Paimon snapshot projections. */
+final class PaimonCacheSizeEstimator {
+ private PaimonCacheSizeEstimator() {
+ }
+
+ static MetaCacheSizeEstimate estimateSnapshotEntry(
+ PaimonSnapshotEntryKey key, PaimonSnapshotCacheValue value) {
+ Table table = value.getSnapshot().getTable();
+ if (!isSupportedTable(table)) {
+ return MetaCacheSizeEstimate.incomplete("unsupported_paimon_table:"
+ + (table == null ? "null" : table.getClass().getName()));
+ }
+ MetaCacheSizeEstimate owned = OwnedObjectSizeEstimator.estimate(key,
value);
+ if (!owned.isComplete()) {
+ return owned;
+ }
+ // Materialize lazyStore before admission. It is then covered by the
version-pinned type
+ // formula below; walking the whole SDK graph would cross
implementation caches/services.
+ ((FileStoreTable) table).store();
+ long bytes = add(owned.getBytes(), estimateTable(
+ table, Collections.newSetFromMap(new IdentityHashMap<>())));
+ bytes = add(bytes,
JvmSizeUtils.shallowSizeOf(MetaCacheSizeEstimate.class));
+ return MetaCacheSizeEstimate.complete(bytes);
+ }
+
+ /**
+ * Count the snapshot-scoped table wrapper and its immutable
schema/options. FileIO, catalog
+ * environment and SDK-internal shared caches remain ownership boundaries.
+ */
+ private static long estimateTable(Table table, Set<RowType>
visitedRowTypes) {
+ if (table == null) {
+ return 0L;
+ }
+ long bytes = JvmSizeUtils.shallowSizeOf(table.getClass());
+ bytes = add(bytes, JvmSizeUtils.sizeOfString(table.name()));
+ long rowTypeBytes = estimateRowType(table.rowType(), visitedRowTypes);
+ bytes = add(bytes, rowTypeBytes);
+ bytes = add(bytes, estimateStringList(table.partitionKeys()));
+ bytes = add(bytes, estimateStringList(table.primaryKeys()));
+ bytes = add(bytes, estimateStringMap(table.options()));
+ bytes = add(bytes,
table.comment().map(JvmSizeUtils::sizeOfString).orElse(0L));
+ if (table instanceof FileStoreTable) {
+ FileStoreTable fileStoreTable = (FileStoreTable) table;
+ bytes = add(bytes, estimateTableSchema(fileStoreTable.schema()));
+ bytes = add(bytes,
JvmSizeUtils.shallowSizeOf(fileStoreTable.location().getClass()));
+ bytes = add(bytes,
JvmSizeUtils.sizeOfString(fileStoreTable.location().toString()));
+ // store() was materialized before this formula was evaluated.
+ FileStore<?> store = fileStoreTable.store();
+ bytes = add(bytes, JvmSizeUtils.shallowSizeOf(store.getClass()));
+ bytes = add(bytes, estimateRowType(store.partitionType(),
visitedRowTypes));
+ // Paimon derives several separately retained RowTypes and small
factories from the
+ // same schema. Count six full-schema equivalents plus wrapper
headroom. Exact field
+ // contracts are pinned by ExternalSdkSizeFieldCoverageTest on
every SDK upgrade.
+ bytes = add(bytes, multiply(6L, estimateRowType(table.rowType(),
+ Collections.newSetFromMap(new IdentityHashMap<>()))));
+ bytes = add(bytes, 16L * 1024L);
+ if (table instanceof FallbackReadFileStoreTable) {
+ FallbackReadFileStoreTable fallback =
(FallbackReadFileStoreTable) table;
+ bytes = add(bytes, estimateTable(fallback.wrapped(),
visitedRowTypes));
+ bytes = add(bytes, estimateTable(fallback.other(),
visitedRowTypes));
+ }
+ }
+ return bytes;
+ }
+
+ private static boolean isSupportedTable(Table table) {
+ if (table == null) {
+ return false;
+ }
+ String className = table.getClass().getName();
+ if
("org.apache.paimon.table.AppendOnlyFileStoreTable".equals(className)
Review Comment:
[P2] Support the privilege wrapper before rejecting the table
A production Paimon table can be a `PrivilegedFileStoreTable`: Doris
explicitly accepts that delegate in `PaimonReaderOptions`, and its
schema/time-travel copies preserve the wrapper. Such a table reaches snapshot
publication still wrapped, but this exact-class allowlist rejects it as
`unsupported_paimon_table` without examining the supported underlying
file-store table. With snapshot weight governance enabled the projection is
then returned once but never cached, so every request reloads and re-enumerates
all partitions. Please handle the approved privilege delegate chain (and
account for its owned wrapper state) and cover it with a weighted-cache test.
--
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]
