morningman commented on code in PR #68842:
URL: https://github.com/apache/doris/pull/68842#discussion_r4236518995
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonCacheSizeEstimator.java:
##########
@@ -98,10 +133,62 @@ private static long estimateFileStoreTable(FileStoreTable
table, Set<Object> vis
((DelegatedFileStoreTable) table).wrapped(), visited));
}
bytes = add(bytes, estimateCompleteOnce(table.schema(), visited));
+ bytes = add(bytes, estimateStoreGrowth(table.schema()));
+ bytes = add(bytes, estimateTableOwnedFileIO(table.fileIO(), visited));
bytes = add(bytes, estimatePath(table.location(), visited));
return add(bytes,
estimateCatalogEnvironment(table.catalogEnvironment(), visited));
}
+ /** Reserves the store and index graph this table builds after admission;
no IO, no store access. */
+ private static long estimateStoreGrowth(TableSchema schema) {
+ long bytes = add(STORE_GROWTH_BASE_BYTES,
+ multiply(schema.options().size(), STORE_GROWTH_OPTION_BYTES));
+ long keys = (long) schema.partitionKeys().size() +
schema.primaryKeys().size() + schema.bucketKeys().size();
+ bytes = add(bytes, multiply(keys, STORE_GROWTH_KEY_BYTES));
+ for (String primaryKey : schema.primaryKeys()) {
+ bytes = add(bytes, JvmSizeUtils.stringSize(primaryKey));
+ }
+ return add(bytes, estimateFieldGrowth(schema.fields(),
STORE_GROWTH_FIELD_BYTES));
+ }
+
+ private static long estimateFieldGrowth(List<DataField> fields, long
perFieldBytes) {
+ long bytes = multiply(fields.size(), perFieldBytes);
+ for (DataField field : fields) {
+ bytes = add(bytes, estimateTypeGrowth(field.type()));
+ }
+ return bytes;
+ }
+
+ private static long estimateTypeGrowth(DataType type) {
+ if (type instanceof RowType) {
+ return estimateFieldGrowth(((RowType) type).getFields(),
+ STORE_GROWTH_FIELD_BYTES +
STORE_GROWTH_NESTED_FIELD_BYTES);
+ }
+ if (type instanceof ArrayType) {
+ return estimateChildTypeGrowth(((ArrayType)
type).getElementType());
+ }
+ if (type instanceof MultisetType) {
+ return estimateChildTypeGrowth(((MultisetType)
type).getElementType());
+ }
+ if (type instanceof VectorType) {
+ return estimateChildTypeGrowth(((VectorType)
type).getElementType());
+ }
+ if (type instanceof MapType) {
+ MapType mapType = (MapType) type;
+ return add(estimateChildTypeGrowth(mapType.getKeyType()),
+ estimateChildTypeGrowth(mapType.getValueType()));
+ }
+ return 0L;
+ }
+
+ private static long estimateChildTypeGrowth(DataType child) {
+ return add(STORE_GROWTH_TYPE_NODE_BYTES, estimateTypeGrowth(child));
+ }
+
+ private static long estimateTableOwnedFileIO(FileIO fileIO, Set<Object>
visited) {
+ return fileIO instanceof RESTTokenFileIO && visited.add(fileIO) ?
REST_TOKEN_FILE_IO_BYTES : 0L;
+ }
Review Comment:
You're right, and my previous reply overstated it: ce25717d42c charged the
allowance only on the `FileStoreTable` path. In Paimon 1.4.2,
`CatalogUtils.loadTable` gives an external table of any kind the catalog's
external FileIO, and `RESTCatalog` binds that to `fileIOFromOptions`, a new
`ResolvingFileIO` per table even with data tokens enabled.
b6db733bc69 charges format, object, Lance and Iceberg tables the same 16 KB
allowance. `tablesOutsideTheFileStoreCarryTheFileIoAllowanceToo` builds each of
the four on its own `ResolvingFileIO` and checks that the weight exceeds the
allowance and, once the FileIO has built its backend, still covers the retained
graph. Without the new charge a format table weighs 1,600 bytes against a
16,408-byte allowance, and the test fails.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergCatalogFactory.java:
##########
@@ -161,14 +163,17 @@ public static Map<String, String>
buildBaseCatalogProperties(Map<String, String>
* {@code io.manifest.cache-enabled} directly — derive it to {@code
"true"} from the FE meta-cache
* spec ({@code meta.cache.iceberg.manifest.*}) using the same {@code
enable && ttl != 0 &&
* capacity != 0} rule. Default-disabled (legacy {@code
DEFAULT_ICEBERG_MANIFEST_CACHE_ENABLE}).
+ * Nothing is derived under a metadata cache weight limit: the SDK cache
keeps up to
+ * {@code io.manifest.cache.max-total-bytes} of manifest content per
FileIO outside that limit.
*/
- private static void appendManifestCacheProperties(Map<String, String>
props, Map<String, String> opts) {
+ private static void appendManifestCacheProperties(Map<String, String>
props, Map<String, String> opts,
+ boolean metaCacheWeightLimited) {
boolean hasExplicitEnabled =
StringUtils.isNotBlank(props.get(CatalogProperties.IO_MANIFEST_CACHE_ENABLED));
copyIfPresent(props, opts,
CatalogProperties.IO_MANIFEST_CACHE_ENABLED);
copyIfPresent(props, opts,
CatalogProperties.IO_MANIFEST_CACHE_EXPIRATION_INTERVAL_MS);
copyIfPresent(props, opts,
CatalogProperties.IO_MANIFEST_CACHE_MAX_TOTAL_BYTES);
copyIfPresent(props, opts,
CatalogProperties.IO_MANIFEST_CACHE_MAX_CONTENT_LENGTH);
- if (!hasExplicitEnabled) {
+ if (!hasExplicitEnabled && !metaCacheWeightLimited) {
Review Comment:
Agreed for the server defaults, fixed in 4728d698666. Under a weight limit,
when the catalog does not set `io.manifest.cache-enabled` itself, the client
properties now carry `false`. `ConfigResponse.merge` starts from the server
defaults and puts the client properties on top, so a REST `/config` default of
`true` no longer turns the cache on.
`metaCacheWeightLimitTurnsTheSdkManifestCacheOff` merges the built properties
with such a response; leaving the key out makes it fail.
Server overrides and per-table config are left alone. The REST protocol
defines overrides as values the server mandates over client configuration, and
`RESTSessionCatalog.tableFileIO` lets a table's config win over the catalog's.
Forcing `false` past them would mean replacing the FileIO the SDK builds, and a
server that mandates the cache has chosen to spend that memory. An
`io.manifest.cache-enabled` set on the catalog still wins, as before.
--
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]
