Skip to content

Fix misleading shutdown interruption warnings - #18939

Merged
JackieTien97 merged 5 commits into
masterfrom
codex/fix-shutdown-interruption-logs
Oct 21, 2026
Merged

JackieTien97 merged 5 commits into
masterfrom
codex/fix-shutdown-interruption-logs

Conversation

@JackieTien97

@JackieTien97 JackieTien97 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

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

  • Suppress the queue-poll warning only when the driver thread has been closed.
  • Classify direct or wrapped execution interruptions after closure as scheduler shutdown, with an English/Chinese abort reason.
  • Continue calling scheduler.toAborted() to release query resources. Keep warnings for unexpected runtime interruptions and non-interruption failures.

Internal metric reporter and failure provenance

  • Publish the reporter's stopped state before cancelling work, covering both node shutdown and reporter reload.
  • Skip new or queued writes and follow-up schema creation/retries once stopping begins; handle the race between task submission and executor shutdown.
  • Preserve interruption provenance from remote write dispatch through FragInstanceDispatchResult, ClusterScheduler, QueryStateMachine, and ExecutionResult. Both interrupted and normal returns from QueryExecution.getStatus() retain that provenance, even if the thread's interrupt flag has been consumed.
  • Atomically store a failure status with its provenance and retain the first recorded status. An interruption marker must not reclassify a different status or hide an unrelated exception.
  • Use explicit provenance to downgrade expected interrupted metric operations during shutdown. Existing read-only, write-process, internal-error, and exception failures remain WARN during reporter reload. Read-only responses during node shutdown are still handled as expected shutdown failures.
  • Preserve RPC status codes and localize the interrupted-dispatch status message through the existing English/Chinese message classes.
  • Serialize reporter start/stop and reject restarting an already stopped executor.

Validation

  • Full clean English reactor build and 47 targeted unit tests passed:
    mvn clean test \
      -Dtest=AbstractDriverThreadShutdownTest,IoTDBInternalLocalReporterTest,DriverTaskTimeoutSentinelThreadTest,QueryExecutionRetryTest,QueryStateMachineTest \
      -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false
  • Full clean Chinese main/test compilation passed:
    mvn clean test-compile -DskipTests -P with-zh-locale
  • mvn spotless:apply -pl iotdb-core/datanode, git diff --check, message-key parity, and format-specifier checks passed.
  • Remote-dispatch regressions use a real scheduled executor, ClusterScheduler, FragmentInstanceDispatcherImpl, AsyncPlanNodeSender wait, and QueryExecution.getStatus(), with the RPC response held by mocked transport. The three no-WARN cases failed before the dispatch-provenance fix and now pass.
  • Coverage includes remote metric writes, auto-creation of timeseries, consumed interrupt flags, pre-existing status/exception failures, status-provenance identity, shutdown/submission races, driver cleanup, normal query timeout, and query retries.
  • The original HA cluster shutdown scenario has not been rerun.

This PR has:

  • Been self-reviewed, including concurrent shutdown and task submission.
  • Added comments explaining non-obvious shutdown handling.
  • Added unit tests for the changed behavior.
  • Included Apache license headers in new test files.
Key changed/added classes or packages
  • Driver scheduling: AbstractDriverThread, DriverTaskAbortedException
  • Fragment dispatch: FragInstanceDispatchResult, FragmentInstanceDispatcherImpl, ClusterScheduler
  • Query state/results: QueryStateMachine, QueryExecution, ExecutionResult
  • Internal metrics: IoTDBInternalLocalReporter
  • Tests: AbstractDriverThreadShutdownTest, IoTDBInternalLocalReporterTest, QueryStateMachineTest

@JackieTien97 JackieTien97 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 JackieTien97 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@JackieTien97
JackieTien97 merged commit ccbc71d into master Oct 21, 2026
37 of 39 checks passed
@JackieTien97
JackieTien97 deleted the codex/fix-shutdown-interruption-logs branch October 21, 2026 02:01
@codecov

codecov Bot commented Oct 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.70175% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.07%. Comparing base (49ba3be) to head (30ecca6).
⚠️ Report is 11 commits behind head on master.

Files with missing lines Patch % Lines
...db/service/metrics/IoTDBInternalLocalReporter.java 68.96% 18 Missing ⚠️
.../db/queryengine/plan/execution/QueryExecution.java 69.23% 4 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant