924060929 commented on code in PR #68283:
URL: https://github.com/apache/doris/pull/68283#discussion_r4059274867
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -397,8 +399,45 @@ public class FrontendServiceImpl implements
FrontendService.Iface {
private ConcurrentHashMap<Long, AtomicInteger>
multiTableFragmentInstanceIdIndexMap =
new ConcurrentHashMap<>(64);
- private final Map<TUniqueId, ConnectContext> proxyQueryIdToConnCtx =
- new ConcurrentHashMap<>(64);
+ private final ProxyQueryRegistry proxyQueryRegistry = new
ProxyQueryRegistry();
+
+ static final class ProxyQueryRegistry {
+ private final Object lock = new Object();
+ private final Map<TUniqueId, ConnectContext> contexts = new
HashMap<>();
+ private final Cache<TUniqueId, Status> pendingCancels =
CacheBuilder.newBuilder()
+ .maximumSize(1_000_000)
+ .expireAfterWrite(30, TimeUnit.MINUTES)
+ .build();
+
+ Runnable register(TUniqueId queryId, ConnectContext context) {
+ Status pendingCancel;
+ synchronized (lock) {
+ contexts.put(queryId, context);
Review Comment:
This finding was valid for the previous cross-FE cancellation expansion.
That expansion has been removed in 870c7c24d72. The PR is now limited to the
StmtExecutor-to-Coordinator publication handoff; FEOpExecutor retry semantics
remain unchanged and should be handled separately.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/MasterOpExecutor.java:
##########
@@ -59,24 +66,52 @@ public MasterOpExecutor(ConnectContext ctx) {
@Override
public void execute() throws Exception {
- super.execute();
- waitOnReplaying();
+ synchronized (executionLock) {
+ if (cancelReason != null) {
+ throw new UserException("Forwarded statement was terminated
before execution: "
+ + cancelReason.getErrorMsg());
+ }
+ executionStarted = true;
+ }
+ try {
+ super.execute();
+ waitOnReplaying(result);
+ } finally {
+ synchronized (executionLock) {
+ executionFinished = true;
+ }
+ }
}
@Override
public void cancel() throws Exception {
- super.cancel();
- waitOnReplaying();
+ cancel(Status.CANCELLED);
+ }
+
+ public void cancel(Status reason) throws Exception {
+ synchronized (executionLock) {
+ if (cancelReason == null) {
+ cancelReason = reason;
+ }
+ if (!executionStarted || executionFinished || cancelForwarded) {
+ return;
+ }
+ cancelForwarded = true;
Review Comment:
This finding was valid for the previous forwarded-cancellation expansion.
The MasterOpExecutor changes have been removed in 870c7c24d72. This PR no
longer changes cancel delivery or retry state.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -1621,29 +1651,34 @@ public void executeAndSendResult(boolean
isOutfileQuery, boolean isSendFields,
// Where the result goes is the protocol's decision, made now, before
the coordinator is
// built: relayed by this frontend through the sender, or left on the
backends for the
// client to pull (context.isReturnResultFromLocal() is false then).
+ throwIfTerminated();
Review Comment:
The generic statement termination gate and OUTFILE-path changes have been
removed in 870c7c24d72. The PR now only transfers a pending terminal status to
a late-published coordinator and prevents that coordinator from dispatching
fragments; OUTFILE cleanup behavior is unchanged and outside this PR.
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -1248,10 +1287,8 @@ private TMasterOpResult
handleForwardCancel(TMasterOpRequest params) throws TExc
if (!params.isSetQueryId()) {
throw new TException("a query id is needed to cancel a query");
}
- ConnectContext context =
proxyQueryIdToConnCtx.get(params.getQueryId());
- if (context != null) {
- context.cancelQuery(new Status(TStatusCode.CANCELLED, "cancel
query by forward request."));
- }
+ proxyQueryRegistry.cancel(params.getQueryId(),
Review Comment:
The cross-FE cancellation and protocol changes have been removed in
870c7c24d72, so this PR no longer translates or transports terminal reasons
between FEs. The scoped local StmtExecutor-to-Coordinator handoff retains the
original Status without a Thrift change.
--
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]
