[XuPeng-SH]

What type of PR is this?

• BUG
• Improvement

Which issue(s) this PR fixes:

Fixes #30737

What this PR does / why we need it:

Cancelling asynchronous CSV/JSON conversion can leave a missing batch result and make export finalization spin indefinitely. Finalization now waits on ordered results or cancellation, releases completed entries, and rejects incomplete closed streams.

ExportConfig owns cancellation and joining of conversion workers, file pipes, the sink, and the per-file cancellation callback. Cleanup preserves the primary error's concrete identity and keeps request metadata alive until all users exit. Ordinary OUTFILE, DATA BRANCH CSV, and stored-result dumping use this cleanup boundary. DATA BRANCH coordination runs outside the converter pool and observes producer cancellation once while draining results.

Stored-result dumping creates its own cancellation context and passes it to the sink. A known header/source error therefore cancels a context-cooperative sink before joining, even if it is waiting on existence checks or admission rather than consuming the pipe. Dump remains synchronous and allocates no unused conversion queue. The parent request, borrowed shared service, process, and saved metadata retain their ownership.

Rebased onto the target branch and aligned all ExportConfig method receiver names, resolving CI staticcheck ST1016 without changing cleanup behavior or suppressing diagnostics.

The 4.2-dev PR also backports #29344 for #29266: unknown-size empty S3 writes publish zero-byte objects, and failed multipart writes reach bounded Abort cleanup. Sequential/parallel Abort retains context values, survives caller cancellation, and has a 30-second deadline. Successful Abort preserves primary error identity. SDK dependencies, nonempty chunk sizing and successful request sequence remain unchanged.

Validation

• Final frontend owning-package normal and full -race -count=1 suites: 1599 root tests passed in each, with one existing skip.
• Rebase preserved all three functional commits. Backend source and dependencies are unchanged; prior backend evidence remains applicable.
• New actual doDumpQueryResult regression covers oversized headers, a source-file read failure after sink startup, parent cancellation, primary error identity, joined sink, and continued shared-service/metadata usability. Retained evidence from the identical regression source records 100 race repetitions per branch; all three scenarios also passed in the post-rebase full race suites. A pre-repair production-function overlay makes the source-error cell fail as expected until independent parent cancellation; the probe cancels and drains itself.
• Existing public SQL lifecycle test passed under race: exact CSV contents, destination failure and same-session reuse. Existing multi-block stored-result dump content test passed.
• Scoped go vet, configured molint, formatting, license-eye and incremental configured golangci-lint passed. Full affected-package lint reports 41 diagnostics: a separate clean b7d2cc7b56 worktree produces the identical complete diagnostic set after normalizing line offsets. No new findings, no ST1016, no suppression. License-eye checked all changed files (7 main / 11 release), 0 invalid; policy unchanged.
• Separate gpt-6.1-sol / xhigh design and final complete-PR review passed with no unresolved material blocker. Local results are distinct from remote CI.
• Retained backend evidence remains applicable: fileservice owning-package suite (299 roots), affected AWS/S3 race tests (51 roots), controlled HTTP Abort deadlines/errors, real SQL empty-object publication, and combined SQL cancellation/prepared reuse. The later unmodified-binary blackbox comparison reproduced the 4.2 baseline hang, while this PR's export lifecycle passed 32/32 timed cancellations with same-session health and correct normal CSV contents. No backend source or dependencies changed in this follow-up.

Local validation artifacts: /home/xupeng/.codex/issue-to-pr/matrixone/30737/rebase-review/ (scope.json, results.json, run-validation.sh, *-normal.jsonl, *-race.jsonl, *-public.jsonl, *-vet.log, *-molint.log, *-lint.log, *-license.log, exit receipts). The release baseline comparison is 42-lint-comparison.json; retained stress/negative controls are in the adjacent review-ci-repair/ directory.

Performance and limits

Retains asynchronous conversion. Blocking finalizer waits replace busy spinning. Stored-result dumping adds one child context; it adds no conversion queue, worker or success-path backend request. No broad throughput or long-soak guarantee is claimed.

Global admission bounds and arbitrary I/O that ignores cancellation remain outside this fix. The separately tracked borrowed-service SQL DATA BRANCH DIFF issue #30755 is unchanged.
Failed S3 writes can wait for the bounded Abort operation; the 30-second cleanup budget does not bound the entire SQL request or arbitrary source readers. Remote reclamation requires a successful Abort. The pre-upload parallel cancellation schedule addressed by main #29312 remains a separate known gap.

Remote CI is pending for the updated revision.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[XuPeng-SH]

mo-bot review · mo-bot-xupeng · superseded

Commit: 2b3f726d4feb6cba0ebccd5fd2e73343f3e67faa

"interrupted"

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: 2b3f726d4feb6cba0ebccd5fd2e73343f3e67faa

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: 2b3f726d4feb6cba0ebccd5fd2e73343f3e67faa

Assessment: request_changes

Reviewed the complete seven-file delta and affected callers. Shared cleanup ownership, blocking ordered consumption, and error-identity preservation are sound improvements. However, stored-result dumping adopts the cleanup boundary without initializing its cancellation ownership, introducing an error-path stall.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1586-7a01-9000-d45e20995cf8 · slot 1 · generation 1

• P2 pkg/frontend/query_result.go:748: Dump a stored result with the CSV header disabled. A result-file read fails while the asynchronous sink awaits a context-aware operation before reading the pipe, such as S3FS.Write's storage.Exists call.
  Impact: doDumpQueryResult assigns the parent context but never initializes exportParam.cancel. The new deferred finish therefore waits in AsyncGroup.Wait without canceling the sink. Closing the pipe cannot release that backend operation, so returning the already-known read error waits for independent parent cancellation, a deadline, or backend completion. Initialize an export-owned cancellable context and pass that child to openNewFile; add a regression covering this entry point.
  Evidence: Source tracing confirms the missing initialization and unconditional sink join. TestReviewStoredDumpCleanupCancelsSink reproduces the dump's initialization using the real export helpers and a cooperative sink: cleanup remains blocked until the parent is separately canceled. Receipt checks/1792574200148160521-4.json records exit 1 and stored-dump-probe.jsonl contains the assertion failure. The control initializes the export and passes ep.ctx to openNewFile; checks/1792574266730144915-4.json records exit 0. The base error defer only closed endpoints and did not synchronously wait.

Validation:

• Verified the clean frozen HEAD, complete patch, all seven changed files, and matching native provenance: 172 input hashes and six build-output hashes.

• Isolation preflight passed. Assigned caches and test-exec were used for all Go test executions.

• Full frontend race suite passed 1,598 root tests with one unrelated skip. Twelve focused roots passed individually and each passed 100 race repetitions, totaling 1,200 stress completions.

• Public SQL lifecycle control passed: exact CSV output, destination failure, and subsequent same-session query. Scoped go vet passed.

• Commands, source fingerprints, exits, logs, and diagnostic overlays were reconciled in /home/xupeng/.local/state/mo-bot/jobs/6fc06aea872b1ba3d2f0d397/attempt-1/review-evidence.json. Final audit receipt checks/1792574506193860357-4.json records exit 0.

Limitations:

• The finding uses a deterministic frontend helper probe with a cooperative fake sink; it is not a public SQL or live S3 fault-injection result.

• CI was pending at the observed snapshot. The evidence packet contained no retained local-validation inventory or separate linked-issue body.

• No independent performance benchmark was established. Source analysis shows ordinary non-export paths bypass the changes; export adds cancellation bookkeeping while removing finalizer spinning and completed-map retention.

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: e6ce020e6fbcf6114df36195ac1b9e635215da67

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: e6ce020e6fbcf6114df36195ac1b9e635215da67

Assessment: request_changes

Reviewed the complete 11-file delta and affected callers. Shared cleanup ownership, blocking ordered consumption, and bounded S3 abort are sound improvements. However, stored-result dumping still lacks cancellation ownership, leaving the previously reported cleanup stall unresolved.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1586-7a01-9000-d45e20995cf8 · slot 1 · generation 1

• P2 pkg/frontend/query_result.go:748: Dump a stored result with CSV headers disabled. A source-file read fails while the asynchronous sink awaits a context-aware operation before reading the pipe, such as S3FS.Write's storage.Exists call.
  Impact: doDumpQueryResult assigns the parent context but never initializes exportParam.cancel. Deferred finish therefore joins the sink without canceling it. Closing the pipe cannot release the backend operation, so returning the known source error waits for separate parent cancellation or backend completion. Initialize an export-owned cancellable context, pass it to openNewFile, and add a regression for this entry point.
  Evidence: Current-head TestReviewStoredDumpCleanupCancelsSink reproduced the stall with real export helpers and a cooperative fake sink: cleanup completed only after separate parent cancellation. checks/1792581896487748739-3.json records exit 1. The control initializes the child context and passes ep.ctx to openNewFile; checks/1792581961298428721-3.json records exit 0. Both logs and diagnostic overlays are retained. The base error defer closed endpoints without synchronously joining the sink.

Validation:

• Verified the clean frozen HEAD and complete patch. Recovered native artifacts matched 172 current-source input hashes and six output hashes.

• Isolation preflight passed. All current Go tests used the assigned caches and test-exec.

• Current-head fileservice race suite passed 299 root tests with six skips; frontend race suite passed 1,598 roots with one skip.

• Seven focused S3 roots passed, followed by 547 adaptive race repetitions across individually named tests.

• Scoped go vet passed for pkg/fileservice, pkg/frontend, and pkg/tests/issues.

• Reconciled retained frontend stress and public-SQL evidence against the earlier revision's identical frontend source; these were not represented as current-head reruns.

• Commands, source fingerprints, exits, logs, and overlays were audited in /home/xupeng/.local/state/mo-bot/jobs/a511774dae52350cf175fd24/attempt-1/review-evidence.json. Final audit receipt checks/1792582115345487424-3.json records exit 0.

Limitations:

• The finding is reproduced through frontend helpers with a cooperative fake sink, not live S3 fault injection. S3 tests use controlled HTTP servers.

• No independent performance benchmark was established. Source analysis shows non-export frontend paths remain unaffected; export adds cancellation bookkeeping while removing spinning and completed-map retention. Failed S3 writes can spend up to 30 seconds aborting.

• CI remained pending in the frozen snapshot. The evidence packet contained no retained local-validation inventory or separate linked-issue bodies.

[XuPeng-SH]

mo-bot review · mo-bot-xupeng · superseded

Commit: e99cb85769c40ff014283c26f4557a50f5649521

"interrupted"

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: e99cb85769c40ff014283c26f4557a50f5649521

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: e99cb85769c40ff014283c26f4557a50f5649521

Assessment: approve

Reviewed all 11 changed files and affected callers. Centralized export cleanup correctly joins workers, sinks and callbacks while preserving borrowed resources and primary errors. The stored-result cancellation fix closes both prior review findings. No actionable defects remain.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1517-7f90-b3f9-c5f9f9e7031b · slot 0 · generation 1

Validation:

• Verified frozen HEAD, unchanged source, successful isolation preflight, and compatible native build inputs and artifacts.

• Current frontend full race suite: 1,599 tests passed; one existing skip.

• Stored-result cancellation regression passed 100 race repetitions, covering header failure, source failure and parent cancellation. Multi-block successful dumping also passed 100 repetitions.

• Current public SQL race test passed exact CSV contents, destination failure and same-session reuse.

• Independently reconciled retained evidence for unchanged code: fileservice full race suite passed 299 tests with six skips; focused backend stress passed 547 repetitions; export lifecycle stress passed 1,200 repetitions.

• Scoped go vet, formatting and diff checks passed. Commands, source fingerprints, exit receipts and log hashes are recorded in /home/xupeng/.local/state/mo-bot/jobs/2d42797aa1f827fcb66a6f55/attempt-1/review-evidence.json.

Limitations:

• Frozen remote CI remained pending. Linked issue bodies were absent; requirements were assessed from the supplied PR description and review history.

• S3 validation used controlled HTTP endpoints, not live cloud storage. No throughput or long-soak benchmark is claimed.

• The change removes finalizer spinning and completed-map-entry retention. It adds export-specific cancellation/join bookkeeping; failed uploads may spend up to 30 seconds in Abort cleanup. Arbitrary noncooperative I/O and the separately documented preexisting gaps remain outside scope.

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: 733a7efe38c3585e50f4104b97b378045c5ed2ce

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: 733a7efe38c3585e50f4104b97b378045c5ed2ce

Assessment: approve

Reviewed the complete 11-file delta, affected callers, tests and supplied review concerns. Shared ExportConfig cleanup restores cancellation and join-before-release invariants without duplicating teardown across entry points. Stored-result dumping now closes the previously reported cancellation gap. Empty S3 writes and bounded multipart abort use appropriate existing owners. No actionable unresolved findings remain.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1586-7a01-9000-d45e20995cf8 · slot 1 · generation 1

Validation:

• Verified the clean frozen HEAD and all 11 candidate file hashes against retained author evidence. Current-candidate frontend runs passed 1,599 root tests in both normal and race modes, with one existing skip; the public SQL content, destination-failure and session-reuse test also passed under race.

• Independently reconciled retained commands, source fingerprints, exit receipts and log hashes: 1,200 export lifecycle race repetitions, 200 stored-result repetitions, 547 S3 repetitions, and the fileservice race suite with 299 passing roots and six skips. Inspected intervening changes before accepting reuse.

• Verified 172 native input hashes and six output hashes against current source and retained build artifacts. No fresh Go execution was needed.

• Scoped go vet, incremental lint and license checks passed. Full lint diagnostics matched the baseline after line-number normalization; molint retained an unchanged routine.go diagnostic.

• Evidence reconciliation passed through check-run: checks/1792593377798346537-3.json. Commands, hashes, logs and source-review rationale are indexed in /home/xupeng/.local/state/mo-bot/jobs/98eef0833d3708e294575985/attempt-1/review-evidence.json.

Limitations:

• CI was queued in the frozen snapshot. Separate linked-issue bodies were not supplied; assessment used the frozen PR requirements and review comments.

• S3 validation uses actual SDK calls against controlled HTTP servers, not live cloud fault injection. Existing integration/OOM skips remain explicit.

• No throughput benchmark is claimed. Source analysis shows removal of finalizer spinning and completed-batch retention, with bounded per-export bookkeeping; non-export frontend paths bypass it. Failed multipart cleanup can take up to 30 seconds.

• Arbitrary noncooperative I/O and explicitly excluded preexisting issues #29312 and #30755 are not represented as fixed.