yonchicy commented on PR #68371:
URL: https://github.com/apache/doris/pull/68371#issuecomment-5865136349
@morningman Thanks for the detailed review. Addressed in
fe8e604b5a8d6f1fdb250368c81f84f9afa7de08.
**P1: Publish the result consumed by the load waiter**
- In the legacy Coordinator, `deltaUrls`, `loadCounters`, `commitInfos`,
and `errorTabletInfos` are now aggregated together with the URL and
first error message before `updateStatus()` can cancel and release the
latch. This remains behind the existing final-report deduplication
gate.
- The four result-container getters now return immutable snapshots taken
under the same lock as their writers. `LoadLoadingTask` no longer
clears the coordinator-owned error tablet list. Nereids also returns
an error-tablet snapshot, alongside its existing counter/commit
snapshots. Later report updates cannot mutate a container the waiter
is already traversing.
- In Nereids, `SingleFragmentPipelineTask.processReportExecStatus()`
runs result aggregation and then the status-update callback inside its
report acceptance section. An already accepted final report is not
aggregated again. Normal completion is notified only afterward.
- Hive/Iceberg/MC commit-data feeding remains after status publication
and transaction-ID resolution. Normal completion still requires that
acceptance to finish. Query cancellation and non-final error handling
remain prompt; the non-final path does not accumulate final counters.
For the final report that triggers the failure, the relevant order is
now:
```text
pass the fragment's final-report/deduplication gate
-> aggregate the load result
-> publish failure status and release load waiters
-> the load task reads result-container snapshots
```
**P2: Hook contract and repeated diagnostics updates**
The diagnostics-only pre-status hook has been removed. Instead,
`doProcessReportExecStatus` receives a status-update callback, with
JavaDoc specifying the aggregation/status/completion ordering and status
handling for incomplete or duplicate reports. Diagnostics are no longer
written once in the hook and again in final-report aggregation for the
same accepted final report, so the old double-update contract is no
longer needed.
**Validation and scope**
The coordinator suites now contain 11 deterministic tests covering full
results at the cancellation boundary, normal completion, completed
report deduplication, snapshot stability across later reports, non-final
errors, external commit-data failure, and query cancellation. Together
with the related fragment and load-job suites, all 44 targeted FE tests
passed with 0 failures, errors, or skips. Checkstyle also passed with 0
violations. No cluster regression test was run locally.
This fixes publication of the current failing final report and safe
container reads. It does not wait for every remaining fragment after
cancellation, or make separate getters one atomic cross-report snapshot.
The deduplication guarantee above applies to already accepted final
reports; existing retry behavior after external commit-data acceptance
fails is unchanged. The PR description has been updated to match.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
