github-actions[bot] commented on code in PR #68842:
URL: https://github.com/apache/doris/pull/68842#discussion_r4233507701
##########
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));
+ }
Review Comment:
[P2] Reserve partial-update merge-factory growth. A wide primary-key table
with `merge-engine=partial-update`, a sequence group covering many columns, and
`fields.default-aggregate-function=last_non_null_value` builds a retained merge
factory on its first scan. That factory adds two per-field maps and a capturing
supplier per aggregated field; subsequent use can populate the retained
RowType's lazy field indexes as well. For sufficiently many fields, this graph
can exceed the fixed 320-byte-per-field reserve after cache admission. Account
for this merge-engine-specific growth or decline weighted retention, and cover
a wide partial-update table in the grown-graph test.
##########
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:
[P1] Account for table-owned FileIO variants. This branch charges only
`RESTTokenFileIO`, but Paimon's default REST path creates a fresh
`ResolvingFileIO` for each table; it copies catalog options and retains backend
IOs in a private map after cache admission. REST external tables use that
resolver even with data tokens enabled, and HMS table loads likewise create a
`HadoopFileIO` with its own lazy map. Because `PaimonMetaCacheCatalog` weighs
tables only once, a weight-limited catalog can retain more heap than
`meta.cache.max-weight` permits. Charge or bound these per-table graphs, or
decline weighted retention, and cover the REST/HMS paths in tests.
--
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]
