[Fix-18570][Master] Detect wrapped CommandDuplicateHandleException in bootstrapError (#18570)

[hellodml]

Was this PR generated or assisted by AI?

YES. The investigation, the one-line change and the unit test were produced with the help of Claude (Anthropic), working from production logs of my own 3.4.2 deployment. Everything was compiled and tested locally before submitting, and the fix direction was proposed by @ruanwenjun in #18570.

Purpose of the pull request

Closes #18570.

CommandDuplicateHandleException is raised inside a CompletableFuture chain and therefore reaches CommandEngine.bootstrapError wrapped in CompletionException. The direct instanceof check cannot see through the wrapper, so the duplicate branch is never taken and the workflow instance is force-failed instead.

Why this matters beyond a wrong log line: forceUpdateWorkflowInstanceState(..., FAILURE) only updates the DB row. It does not remove the execution from workflowRepository nor deregister its event bus. The still-running first execution then has a FAILURE row underneath it:

UnsupportedOperationException: The WorkflowInstance: 516982 state is FAILURE, no need to notify

so it can never complete and never leaves the in-memory repository. Since MasterServerLoadProtection reads

int currentWorkflowInstanceCount = workflowRepository.getAll().size();

the count can no longer fall and CommandEngine keeps refusing to consume commands. In our production incident the logged count sat at exactly 25 against a limit of 20 for two hours (25 x 7194 log lines in a single hour, no other value), 553 of the 573 workflow instances created in that window never got a single task instance row, and only a restart recovered it.

Brief change log

• CommandEngine.bootstrapError: use ExceptionUtils.throwableOfType(...) instead of instanceof so the exception is still recognised when wrapped. ExceptionUtils was already imported in this file, so no new import is required.
• Add CommandEngineTest covering the wrapped, doubly-nested and unwrapped cases.

Verify this pull request

Covered by the new unit test. Reverting only the production line makes exactly the two wrapped cases fail while the unwrapped case still passes:

with fix:     Tests run: 3, Failures: 0, Errors: 0    BUILD SUCCESS
without fix:  Tests run: 3, Failures: 2, Errors: 0    BUILD FAILURE
              CommandEngineTest.bootstrapErrorShouldNotFailWorkflowWhenDuplicateExceptionIsWrapped
              CommandEngineTest.bootstrapErrorShouldNotFailWorkflowWhenDuplicateExceptionIsNestedTwice

Verified against the dev branch with Maven 3.9.6 / JDK 8u492 (mvn -pl dolphinscheduler-master -am install -DskipTests, then mvn -pl dolphinscheduler-master test -Dtest=CommandEngineTest).

The failure is a Mockito verification failure on workflowInstanceDao.forceUpdateWorkflowInstanceState, i.e. it points directly at the workflow being wrongly force-failed.

Notes for reviewers

• bootstrapError is private, so the test invokes it through ReflectionTestUtils. If that is not acceptable here, dropping private and marking it visible-for-testing works equally well and needs no test change.
• transactionTemplate is mocked to run its callback inline. Without that the non-duplicate branch throws NPE instead of producing a clear verification failure; the test still distinguishes fixed from unfixed either way.
• This change only addresses the duplicate command being misclassified. I have not verified whether it also resolves IllegalStateException: WorkflowExecuteRunnable(...) already registered (reproducible by re-running individual tasks of a finished workflow instance in quick succession), nor whether executions already wedged in workflowRepository get cleaned up.

[boring-cyborg]

Thanks for opening this pull request! Please check out our contributing guidelines. (https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md)

[ruanwenjun]

LGTM

[SbloodyS]

+1

[boring-cyborg]

Awesome work, congrats on your first merged pull request!