github-actions[bot] commented on code in PR #66914:
URL: https://github.com/apache/doris/pull/66914#discussion_r3831191513
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java:
##########
@@ -896,13 +901,50 @@ boolean isUserSessionEnabled() {
&&
IcebergCatalogProperties.TYPE_REST.equals(catalogProps.getFlavor());
}
- private Catalog getOrCreateCatalog() {
- if (icebergCatalog == null) {
- synchronized (this) {
- if (icebergCatalog == null) {
- icebergCatalog = createCatalog();
- }
+ /**
+ * Closes a table's FileIO when the cached raw table owns it. Glue and
S3Tables create a per-table
+ * S3FileIO; REST tables are closed only when they do not share the
catalog-level FileIO. Other catalog
+ * flavors are left untouched because they may share a catalog-level
FileIO.
+ */
+ private Runnable cachedTableCleanup(Table table) {
+ return cachedTableCleanup(table, catalogProps.getFlavor());
+ }
+
+ static Runnable cachedTableCleanup(Table table, String flavor) {
+ if (table == null) {
+ return () -> { };
+ }
+ boolean tableOwned = false;
+ try {
+ if (IcebergCatalogProperties.TYPE_GLUE.equals(flavor)
+ || IcebergCatalogProperties.TYPE_S3_TABLES.equals(flavor))
{
+ tableOwned = true;
+ } else if (IcebergCatalogProperties.TYPE_REST.equals(flavor)
+ && table.io() instanceof SupportsStorageCredentials) {
+ tableOwned = !((SupportsStorageCredentials)
table.io()).credentials().isEmpty();
Review Comment:
[P1] Treat config-only REST FileIOs as table-owned. Iceberg 1.10.1's
RESTSessionCatalog reuses the catalog IO only when the table response has empty
config and credentials; a nonempty response config with no credentials still
creates and tracks a distinct per-table IO. This predicate sees the empty
credential list and returns a no-op cleanup, so cache eviction and direct
statement/transaction completion leave that IO to the weak tracker/GC or
catalog teardown, and repeated reloads can retain client graphs. Please base
ownership on the actual REST load decision/IO identity and cover a response
with nonempty config plus empty credentials.
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/StreamingInsertTask.java:
##########
@@ -182,15 +182,21 @@ public void cancel(boolean needWaitCancelComplete) {
}
@Override
- public void closeOrReleaseResources() {
- if (null != stmtExecutor) {
+ public synchronized void closeOrReleaseResources() {
+ ConnectContext taskContext = ctx;
+ try {
+ if (taskContext != null && taskContext.getStatementContext() !=
null) {
+ taskContext.getStatementContext().close();
Review Comment:
[P1] Drain query-finish callbacks when the planning-only attempt fails.
baseCommand.initPlan(..., false) can reach a plugin scan and register both the
read-transaction release and scope-close callbacks under this attempt's query
ID. If planning or rewriting then fails before executeSingleInsert, this close
only drains StatementContext; no unregisterQuery/runAndClear occurs, and the
retry uses a fresh query ID, leaving the old callbacks and a transactional Hive
read lock retained indefinitely. Please run the normal query-id terminal path
after the worker quiesces and add a pre-execution failure/retry test with a
recording callback.
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/StreamingInsertTask.java:
##########
@@ -182,15 +182,21 @@ public void cancel(boolean needWaitCancelComplete) {
}
@Override
- public void closeOrReleaseResources() {
- if (null != stmtExecutor) {
+ public synchronized void closeOrReleaseResources() {
Review Comment:
[P1] Quiesce the attempt before closing it, and guarantee the same cleanup
after STOP. PAUSE can reach this line while before()/initPlan is still running:
stmtExecutor is not published until planning finishes, so cancel(true) has
nothing to wait on, and the worker can resume after close() and publish into
the already-closed connector scope. Conversely, STOP only calls
cancelAllTasks(false); clearRunningStreamTask is PAUSED-only and the canceled
worker skips its finally cleanup, so the scope, fields, manager entry, and
worker thread-local are never released. Please add an attempt-completion
handoff so PAUSE/STOP cancellation is followed by owner-thread terminal cleanup
exactly once, with blocked-planning tests for both statuses.
--
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]
