github-actions[bot] commented on code in PR #68342:
URL: https://github.com/apache/doris/pull/68342#discussion_r4078770116
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/OlapInsertExecutor.java:
##########
@@ -233,7 +233,7 @@ protected void onComplete() throws UserException {
} catch (Exception abortTxnException) {
LOG.warn("errors when abort txn. {}",
ctx.getQueryIdentifier(), abortTxnException);
}
- } else if
(Env.getCurrentGlobalTransactionMgr().commitAndPublishTransaction(
+ } else if
(Env.getCurrentGlobalTransactionMgr().commitAndPublishTransactionWithRetry(
Review Comment:
[P1] Route remote Doris INSERT commits through the FE retry owner
This switches only the local-table executor. For a
`RemoteDorisExternalTable`, `RemoteOlapInsertExecutor` overrides `onComplete`
and calls `FeServiceClient.commitRemoteTxn`; the target's
`FrontendServiceImpl.commitRemoteTxnImpl` still invokes the six-argument cloud
single-attempt overload. The client retries transport failures and `NOT_MASTER`
only, so a returned `DELETE_BITMAP_LOCK_ERR` becomes `ANALYSIS_ERROR`, then a
generic `UserException`, and the source aborts the remote transaction. As a
result, INSERT into a remote Doris cloud MOW table still fails on its first
transient bitmap error. Please route that target-FE endpoint through this retry
entry (or preserve and retry the application status) and add remote-path
coverage.
##########
fe/fe-core/src/main/java/org/apache/doris/transaction/GlobalTransactionMgrIface.java:
##########
@@ -124,6 +124,16 @@ default boolean commitAndPublishTransaction(DatabaseIf db,
List<Table> tableList
txnCommitAttachment);
}
+ // FE-driven inserts own the cloud delete bitmap retries. BE-driven loads
retry at the caller
+ // and must continue to use the single-attempt overloads that accept an
attachment.
+ default boolean commitAndPublishTransactionWithRetry(DatabaseIf db,
List<Table> tableList, long transactionId,
Review Comment:
[P1] Give explicit-transaction INSERTs an FE retry owner
The ownership rule introduced here still misses transaction-model INSERT.
`InsertIntoTableCommand` selects `OlapTxnInsertExecutor`, which only
accumulates `SubTransactionState`; SQL `COMMIT` later calls the separate
four-argument cloud overload from `TransactionEntry`. That overload performs
one delete-bitmap/commit attempt, and `TransactionEntry` immediately aborts the
transaction when it propagates `DELETE_BITMAP_LOCK_ERR`. Thus `BEGIN; INSERT
...; COMMIT` on a cloud MOW table still fails on the first transient lock or
calculation error, with no BE or FE layer left to retry. Please add equivalent
bounded retry ownership for the subtransaction commit path and cover an
injected first-attempt failure.
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/transaction/CloudGlobalTransactionMgr.java:
##########
@@ -1644,7 +1653,7 @@ public boolean commitAndPublishTransaction(DatabaseIf db,
List<Table> tableList,
while (true) {
try {
res = commitAndPublishTransaction(db, tableList,
transactionId, tabletCommitInfos, timeoutMillis,
- null);
+ txnCommitAttachment, streamUpdateInfos);
Review Comment:
[P1] Preserve the retryable error across the streaming callback failure path
When this attempt carries a `StreamingTaskTxnCommitAttachment`,
`beforeCommitted` retains the streaming-job write lock. A real MetaService
`LOCK_EXPIRED` is then converted to `DELETE_BITMAP_LOCK_ERR` before `commitTxn`
returns a `TransactionState`, but `executeCommitTxnRequest` still calls
`StreamingInsertJob.afterCommitted(null, false)`. That callback dereferences
the null state, so an unchecked exception replaces the retryable
`UserException`; this loop never retries, and the retained reentrant lock is
not paired before the outer streaming-task retry. The new test stubs the entire
seven-argument attempt and therefore misses this boundary. Please make the
failed callback cleanup tolerate `txnOperated == false` without masking the
original error, and cover a registered streaming callback plus `LOCK_EXPIRED`.
--
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]
