924060929 commented on code in PR #66914:
URL: https://github.com/apache/doris/pull/66914#discussion_r3838735487
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java:
##########
@@ -271,7 +273,8 @@ public IcebergConnector(Map<String, String> properties,
ConnectorContext context
||
IcebergScanPlanProvider.restVendedCredentialsEnabled(this.properties))
? null
: new IcebergTableCache(
- resolveTableCacheTtlSecond(this.properties),
DEFAULT_TABLE_CACHE_CAPACITY);
+ resolveTableCacheTtlSecond(this.properties),
DEFAULT_TABLE_CACHE_CAPACITY,
+ this::cachedTableCleanup, catalogResourceTracker);
Review Comment:
已修复。写事务现在持有精确 catalog generation lease 直到 commit/rollback/close；其他可变操作使用
operation-owned table 并在完成后释放表级 FileIO。相关写事务与 rotation/reset 测试已覆盖。
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java:
##########
@@ -3019,16 +3020,24 @@ private Table resolveTable(ConnectorSession session,
IcebergTableHandle handle)
// Resolve the per-request ops before the auth scope so a session=user
fail-closed surfaces verbatim (it
// re-validates the credential even on a scope hit).
IcebergCatalogOps ops = catalogOpsResolver.apply(session);
- Table raw = IcebergStatementScope.sharedTable(session,
handle.getDbName(), handle.getTableName(), () -> {
+ Supplier<Table> directLoader = () -> {
try {
- return context == null
- ? loadRawTable(ops, handle)
- : context.executeAuthenticated(() -> loadRawTable(ops,
handle));
+ return context == null ? ops.loadTable(handle.getDbName(),
handle.getTableName())
+ : context.executeAuthenticated(
+ () -> ops.loadTable(handle.getDbName(),
handle.getTableName()));
} catch (Exception e) {
throw IcebergExceptionUtils.wrapTableLoadFailure(
handle, e, "Failed to load table for scan, error
message is:");
}
- });
+ };
+ Table raw = tableCache == null
Review Comment:
已修复。禁用跨查询 table cache 时，statement-owned table 也通过 tracker 获取并持有 generation
lease，statement scope 关闭时同时释放表和 catalog generation；connector close/401 rotation
用例已覆盖。
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java:
##########
@@ -1525,24 +1568,31 @@ public java.util.logging.Logger getParentLogger()
throws java.sql.SQLFeatureNotS
@Override
public void close() throws IOException {
+ // Release cache-owner references first. The resource tracker keeps
the catalog generation alive until
+ // every table owner that was loaded through it has also released its
last statement borrower.
+ invalidateAll();
Review Comment:
已修复。connector close 与 catalog/table publication 已串行化，close 后的 load/build
不能再发布；晚到资源会直接退休。对应 cache-load 与 catalog-build 竞态测试已覆盖。
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergWritePlanProvider.java:
##########
@@ -1054,7 +1063,13 @@ private Table resolveTable(ConnectorSession session,
IcebergTableHandle handle)
throw new DorisConnectorException("Failed to load iceberg
table "
+ handle.getDbName() + "." + handle.getTableName() +
": " + e.getMessage(), e);
}
- });
+ };
+ return resourceTracker == null
+ ? IcebergStatementScope.sharedWritableTable(
+ session, handle.getDbName(), handle.getTableName(),
loader)
+ : IcebergStatementScope.sharedTrackedWritableTable(
Review Comment:
这条评论要求修复 ConnectorRewriteGroupTask/StatementContext
的通用终止所有权，属于既有任务生命周期问题，不属于本 PR 仅处理 Hudi/Iceberg 资源关闭与泄露的范围。当前 head 已撤回对应通用任务改动，本
PR 忽略该问题。
--
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]
