yujun777 commented on code in PR #68662:
URL: https://github.com/apache/doris/pull/68662#discussion_r4141460240
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertOverwriteTableCommand.java:
##########
@@ -261,37 +286,56 @@ public void run(ConnectContext ctx, StmtExecutor
executor) throws Exception {
// When inserting, BE will call to replace partition by
FrontendService. FE will register new temp
// partitions and return. for transactional, the replacement
will really occur when insert successed,
// i.e. `insertInto` finished. then we call taskGroupSuccess
to make replacement.
- insertIntoAutoDetect(ctx, executor, taskId);
+ InsertCommandContext insertCtx = insertIntoAutoDetect(ctx,
executor, taskId);
+ if (isCancelled.get() && insertCtx.hasCommittedNothing()) {
+ // The load committed no row, so the cancellation still
has everything to take back: the
+ // catch drops the group's temp partitions (there are
none), and the statement fails
+ // rather than publishing a replacement the client
cancelled.
+ throw cancelledBeforeTheRowsWereCommitted("after a load
that committed nothing", ctx);
+ }
insertOverwriteManager.taskGroupSuccess(taskId, (OlapTable)
targetTable, isForceDropPartition());
} else {
// it's overwrite table(as all partitions) or specific
partition(s)
List<String> tempPartitionNames =
InsertOverwriteUtil.generateTempPartitionNames(partitionNames);
+
cancelTheOverwriteAt(DEBUG_POINT_CANCEL_BEFORE_THE_INSERT_OF_AN_OVERWRITE,
targetTable);
if (isCancelled.get()) {
- LOG.info("insert overwrite is cancelled before
registerTask, queryId: {}",
- ctx.getQueryIdentifier());
- return;
+ // Nothing durable happened: no task is registered, no
temp partition exists, no row was
+ // written and nothing was committed. The statement is a
plain failure, like the one the
+ // inner insert reports when it is cancelled, rather than
the success of an overwrite that
+ // did not run.
+ throw cancelledBeforeTheRowsWereCommitted("before
registerTask", ctx);
}
taskId = insertOverwriteManager.registerTask(targetTable,
tempPartitionNames);
if (isCancelled.get()) {
- LOG.info("insert overwrite is cancelled before
addTempPartitions, queryId: {}",
- ctx.getQueryIdentifier());
- // not need deal temp partition
- insertOverwriteManager.taskSuccess(taskId);
- return;
+ // The catch below takes the registration back; no temp
partition exists yet, so there is
+ // nothing else to drop.
+ throw cancelledBeforeTheRowsWereCommitted("before
addTempPartitions", ctx);
}
InsertOverwriteUtil.addTempPartitions(targetTable,
partitionNames, tempPartitionNames);
if (isCancelled.get()) {
- LOG.info("insert overwrite is cancelled before insertInto,
queryId: {}", ctx.getQueryIdentifier());
- insertOverwriteManager.taskFail(taskId);
- return;
+ // The catch below drops the temp partitions this
cancelled statement created.
+ throw cancelledBeforeTheRowsWereCommitted("before
insertInto", ctx);
}
// todo: need to refresh remote target table after add temp
partitions
- insertIntoPartitions(ctx, executor, tempPartitionNames,
wholeTable);
Review Comment:
Fixed, on the same boundary the previous commit drew. The insert context now
records that its transaction committed (set by each executor where its own
commit happens -- OLAP, remote OLAP, external/plugin -- via
`AbstractInsertExecutor#markCommitted`), separately from the response state the
insert leaves behind. The overwrite reads it in both places it decides:
- the window's cancellation is honoured only when nothing was committed;
- `runInsertCommand` no longer stops the overwrite on an error state whose
rows are committed: the swap publishes them, and the client keeps the error the
session asked for (`insert_visible_timeout_return_mode=error`), which is a
statement about visibility rather than about whether the overwrite ran.
Verified with the publish daemon blocked
(`PublishVersionDaemon.stop_publish`, 10s) and
`insert_visible_timeout_ms=1000`: the statement returns the visibility-timeout
error, the FE log records `insert overwrite continues over an error state whose
rows are committed`, and the row the overwrite read becomes visible once
publish resumes. The suite's new `dst_after_the_publish_timeout` case asserts
exactly that -- with the previous code the table keeps its old rows and the
wait fails.
One boundary worth naming, since the response stays the error the session
asked for: a client that retries the overwrite after that error gets a partial
overwrite, because the re-run reads from the offset the first attempt advanced.
That is the same contract `insert_visible_timeout_return_mode=error` has for a
plain `INSERT`, and making the retry safe belongs to the durable fix (the swap
as a committed action of the insert transaction) rather than to this 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]
