Skip to content

Fix write-back sink history failure state handling - #19056

Open
Caideyipi wants to merge 3 commits into
apache:masterfrom
Caideyipi:codex/write-back-sink-history-failure
Open

Caideyipi wants to merge 3 commits into
apache:masterfrom
Caideyipi:codex/write-back-sink-history-failure

Conversation

@Caideyipi

Copy link
Copy Markdown
Collaborator

Summary

  • preserve source-specific database synonym keys during pipe parameter validation
  • persist STOPPED status and exception details when DataNode pipe construction fails, including procedure retry and recovery paths
  • keep SHOW PIPES output stable for stopped pipes and add regression coverage for validation, persistence, status reporting, and automatic splitting

Validation

  • Spotless formatting completed with the integration-test profile
  • targeted PipeParameterValidator tests: 5 passed
  • targeted DataNode tests: 23 passed
  • targeted ConfigNode tests: 39 passed
  • final DataNode/ConfigNode reactor test build: passed
  • the automatic-splitting integration test was attempted but could not start because an optional launcher dependency was unavailable; this test is not counted as passing

@Caideyipi

Copy link
Copy Markdown
Collaborator Author

Independent review of commit c98168f found one confirmed P2 issue and one test-logic issue.

Confirmed P2 — full PipeMetaSync loses top-level DataNode failures:

  • PipeMetaSyncProcedure.java:215-220 calls the overload recordDataNodePushPipeMetaExceptions(respMap) without a fallback pipe.
  • A top-level exception in DataNodeInternalRPCServiceImpl.pushPipeMeta (:1395-1398) returns PIPE_PUSH_META_ERROR without exceptionMessages or a status message.
  • PipeTaskInfo.java:1173-1191 sets hasException but, with no fallback, continues without recording STOPPED, isStoppedByRuntimeException, or a node exception.

As a result, a full metadata sync can keep retrying while SHOW PIPES still reports RUNNING and an empty ExceptionMessage, and the failure is not persisted for automatic restart/recovery. The full-sync path needs an attribution/fallback strategy for responses that cannot identify a pipe, plus coverage for the empty-details and retry/persistence cases.

Confirmed test-logic issue (fixed): IoTDBPipeAutoSplitIT supplied user=root for the write-back sink but omitted password, which caused CI error 701. Commit e759aab86b3b9a7e3c0eb5f82199c28a7fd987ba adds password=root. The targeted IoTDBPipeAutoSplitIT run completed 2/2 tests with BUILD SUCCESS; the PR remains draft.

Remaining test gap: PipeMetaSyncProcedureTest covers a named exception message, but does not cover the top-level PIPE_PUSH_META_ERROR response with no exceptionMessages, which is the confirmed failure path above.

@Caideyipi

Copy link
Copy Markdown
Collaborator Author

Independent re-review of PR #19056 at e759aab86b3b9a7e3c0eb5f82199c28a7fd987ba found two actionable P2 behaviors, both fixed in 31d9b949d2947f09fca8ab0cf7346e7ac330f2f8:

  • A full metadata sync receiving PIPE_PUSH_META_ERROR without exception details did not assign the failure to any pipe, so STOPPED, the runtime-exception stop flag, the DataNode exception message, and consensus persistence were missing. The unscoped response is now applied to all pipes during full sync.
  • If a DataNode push failure was recorded in memory but its consensus write failed, a retry that then received a successful push could finish without persisting the earlier failure. The retry path now flushes pending pipe-meta exceptions before pushing again, including Create, Alter, Start, and full-sync procedures.

Regression coverage was added for full-sync unscoped errors and persistence retries in full-sync, Create, and Start. Verification: the focused remote Maven reactor completed with BUILD SUCCESS; Surefire reports 36 tests, 0 failures/errors/skips, including PipeMetaSync, Create, Start, Abstract procedure, and auto-restart tests. git diff --check is clean. The PR remains OPEN/DRAFT as requested.

@Caideyipi

Caideyipi commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

Fresh independent re-review completed at head 31d9b94. I inspected the complete PR diff and relevant procedure, persistence, parameter, SHOW PIPES, retry, and auto-split paths. I found no actionable source-behavior or test-logic issues, so no fixes were needed in this round.

Previously recorded verification: the updated IoTDBPipeAutoSplitIT passed 2/2 tests, and a focused Pipe/ConfigNode test run at this head passed 36 tests with 0 failures, errors, or skips. GitHub checks still include pending jobs; pending checks are not counted as passed.

@Caideyipi
Caideyipi marked this pull request as ready for review October 30, 2026 07:37
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