Skip to content

Fix data exchange failure propagation - #18468

Draft
JackieTien97 wants to merge 1 commit into
apache:masterfrom
JackieTien97:ty-fix-sink-failure-propagation
Draft

Fix data exchange failure propagation#18468
JackieTien97 wants to merge 1 commit into
apache:masterfrom
JackieTien97:ty-fix-sink-failure-propagation

Conversation

@JackieTien97

Copy link
Copy Markdown
Contributor

Description

What changed

  • Fail the receiving fragment instance synchronously when SourceHandle cannot reserve memory, then rethrow the original error through the existing Thrift error path.
  • Notify the upstream fragment instance of a terminal sink-channel failure before updating the remaining-channel count.
  • Distinguish abnormal channel termination from normal completion so a failed or aborted channel cannot close the sink handle and mark the fragment as finished.
  • Serialize ShuffleSinkHandle.close() and abort() termination ownership to prevent mixed terminal callbacks under races.
  • Add regression tests for receiver-side memory failures, three failed RPC attempts, multi-channel accounting, and concurrent close/abort calls.

Root cause

The last failed sink channel previously decremented the channel count before invoking its failure callback. The count reaching zero closed the ShuffleSinkHandle, which could mark the upstream fragment instance as finished before the failure was recorded. On the receiver, an exception raised while reserving memory escaped the RPC without notifying the target fragment instance, so the query could remain active until timeout. Normal and abnormal cleanup also shared the same close path, allowing close and abort callbacks to race.

Compatibility and impact

Queries now record the receiver-side failure before returning the RPC error and fail promptly after terminal upstream retries instead of waiting for query timeout. The Thrift interface and wire schema are unchanged, so this remains compatible with rolling upgrades.

Verification

  • mvn test -pl iotdb-core/datanode -Dtest=MPPDataExchangeManagerTest,SourceHandleTest,SinkChannelFailurePropagationTest,ShuffleSinkHandleTest,SinkChannelTest -DfailIfNoTests=false
  • Result: 18 tests run, 0 failures, 0 errors, 0 skipped.

This PR has:

  • been self-reviewed.
  • added comments explaining the intent of non-obvious concurrent cleanup logic.
  • added or updated unit tests to cover the new failure and race paths.

Key changed/added classes
  • MPPDataExchangeManager.ISinkChannelListenerImpl
  • SourceHandle
  • ShuffleSinkHandle
  • SinkChannelFailurePropagationTest

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