Repository navigation
Fix misleading shutdown interruption warnings - #18939
Conversation
JackieTien97
left a comment
There was a problem hiding this comment.
Found one reproducible P2 issue: reporter reload can suppress an existing, unrelated write failure because the worker's interrupt flag is treated as sufficient evidence that any returned status was caused by shutdown. Please address the inline finding before merging.
Validation: a clean reactor build through the DataNode module and all 27 targeted tests passed. An additional focused reproduction using a real scheduled executor and the actual QueryExecution.getStatus() failed both assertions requiring existing SYSTEM_READ_ONLY and WRITE_PROCESS_ERROR results to remain WARN; the interruption-only control passed. The original HA cluster shutdown scenario was not rerun.
JackieTien97
left a comment
There was a problem hiding this comment.
Rechecked c937b80. The previous finding about hiding pre-existing failures during reporter reload is fixed: all three original reproduction cases now pass, and the clean reactor build through the DataNode module plus all 33 targeted tests passed.
One new P2 regression remains in interruption classification for remote metric writes. A focused reproduction using a real scheduled executor and the actual ClusterScheduler, FragmentInstanceDispatcherImpl, AsyncPlanNodeSender wait, and QueryExecution.getStatus() produces a reporter WARN after shutdownNow(). The same reproduction passes against the corresponding classes from b352989, where that reporter message is DEBUG. RPC transport was mocked; the HA cluster scenario was not rerun. Please address the inline finding before merging.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #18939 +/- ##
============================================
+ Coverage 46.79% 47.07% +0.28%
- Complexity 713 729 +16
============================================
Files 5537 5539 +2
Lines 404649 405207 +558
Branches 52904 53028 +124
============================================
+ Hits 189338 190734 +1396
+ Misses 215311 214473 -838 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description
Normal DataNode shutdown interrupts query scheduler waits and internal metric writes. These expected interruptions could produce
[ExecuteFailed]or metric-write/timeseries-creation WARN logs. Handle shutdown interruptions explicitly while preserving task cleanup and warnings for genuine failures.Query scheduler
scheduler.toAborted()to release query resources. Keep warnings for unexpected runtime interruptions and non-interruption failures.Internal metric reporter and failure provenance
FragInstanceDispatchResult,ClusterScheduler,QueryStateMachine, andExecutionResult. Both interrupted and normal returns fromQueryExecution.getStatus()retain that provenance, even if the thread's interrupt flag has been consumed.Validation
mvn clean test \ -Dtest=AbstractDriverThreadShutdownTest,IoTDBInternalLocalReporterTest,DriverTaskTimeoutSentinelThreadTest,QueryExecutionRetryTest,QueryStateMachineTest \ -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=falsemvn spotless:apply -pl iotdb-core/datanode,git diff --check, message-key parity, and format-specifier checks passed.ClusterScheduler,FragmentInstanceDispatcherImpl,AsyncPlanNodeSenderwait, andQueryExecution.getStatus(), with the RPC response held by mocked transport. The three no-WARN cases failed before the dispatch-provenance fix and now pass.This PR has:
Key changed/added classes or packages
AbstractDriverThread,DriverTaskAbortedExceptionFragInstanceDispatchResult,FragmentInstanceDispatcherImpl,ClusterSchedulerQueryStateMachine,QueryExecution,ExecutionResultIoTDBInternalLocalReporterAbstractDriverThreadShutdownTest,IoTDBInternalLocalReporterTest,QueryStateMachineTest