924060929 commented on code in PR #66913:
URL: https://github.com/apache/doris/pull/66913#discussion_r4056308402
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/rewrite/RewriteGroupTask.java:
##########
@@ -186,6 +212,9 @@ private void executeGroup(ConnectContext taskConnectContext,
StatementBase taskParsedStmt) throws Exception {
// Step 1: Create stmt executor
stmtExecutor = new StmtExecutor(taskConnectContext, taskParsedStmt);
+ if (isCanceled.get()) {
Review Comment:
Fixed on the current head. RewriteGroupTask publishes
insertExecutor.getCoordinator() to StmtExecutor immediately after assigning the
transaction id and before executeSingleInsert(), so cancellation reaches
running BE rewrite groups.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalCatalog.java:
##########
@@ -173,17 +240,108 @@ protected List<String>
listTableNamesFromRemote(SessionContext ctx, String dbNam
}
@Override
- public void onClose() {
+ public synchronized void onClose() {
+ ThreadPoolExecutor retiredExecutor = threadPoolWithPreAuth;
+ threadPoolWithPreAuth = null;
super.onClose();
- if (null != catalog) {
- try {
- if (catalog instanceof AutoCloseable) {
- ((AutoCloseable) catalog).close();
- }
- catalog = null;
- } catch (Exception e) {
- LOG.warn("Failed to close iceberg catalog: {}", getName(), e);
+ Catalog retiredCatalog = catalog;
+ catalog = null;
+ resourceTracker.retireCurrent(() -> {
Review Comment:
Fixed on the current head. IcebergExternalCatalog.onClose() removes the
Iceberg cache group before resourceTracker.retireCurrent(); the final
catalog/executor teardown callback is therefore installed only after cache
removal has returned and its removal locks have been released.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java:
##########
@@ -164,10 +236,21 @@ public void onClose() {
metadataOps.close();
metadataOps = null;
}
- if (null != icebergMetadataOps) {
- icebergMetadataOps.close();
- icebergMetadataOps = null;
- }
+ icebergResourceTracker.retireCurrent(() -> {
Review Comment:
Fixed on the current head. HMSExternalCatalog.onClose() removes the
HMS-Iceberg cache group before entering synchronized catalog cleanup and before
retiring the tracker, preserving lifecycle-stripe -> catalog-monitor order and
keeping final teardown off the cache-removal lock stack.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ExecuteCommand.java:
##########
@@ -72,6 +72,10 @@ public <R, C> R accept(PlanVisitor<R, C> visitor, C context)
{
@Override
public void run(ConnectContext ctx, StmtExecutor executor) throws
Exception {
+ StatementContext statementContext = ctx.getStatementContext();
Review Comment:
Fixed on the current head/base. ExecuteCommand.run() has one execution-local
StatementContext from preparedStmtCtx.nextStatementContext(); the duplicate
local declaration is gone. The rebased FE compiles and IcebergScanNodeTest
passes 100/100.
--
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]
