zclllyybb commented on issue #65582: URL: https://github.com/apache/doris/issues/65582#issuecomment-4966669309
Breakwater-GitHub-Analysis-Slot: slot_19cb3209792b This content is generated by AI for reference only. Initial triage for apache/doris#65582. Summary: - Live issue metadata matches the report: the issue is open, has no labels/comments at the time of triage, and a matching open PR exists: #65584, `[fix](load_stream) fix receiver-side stream leak race on close_load`. - I verified the code path on current `upstream/master` (`052880662b061000694f338d682c741f947353ac`) and current `upstream/branch-4.0` (`54e9da0b631ba8e77510d7b1704319d4bc58fa56`). I did not run the docker reproduction locally. - The receiver-side race described in the issue is real from code inspection. On master, `be/src/load/channel/load_stream.cpp` handles `CLOSE_LOAD` by calling `close()` under `_lock`, then doing `_report_result(..., eos=true)` outside the lock, then reacquiring `_lock` to register an incremental stream in `_closing_stream_ids` or close a non-incremental stream, and finally draining `_closing_stream_ids` if the earlier `close()` result said all streams had closed. Branch-4.0 has the same structure in `be/src/runtime/load_stream.cpp`. - The bad interleaving is credible: a non-last incremental stream can be counted by `close()` but delayed before `_closing_stream_ids.push_back(id)`. The last stream can then observe all `CLOSE_LOAD`s, drain and clear the current list, and return. When the delayed stream later registers itself, no later thread is guaranteed to close it, so `on_closed` never fires on that stream and the sender-side close wait can hang until the load timeout. - The related `Stream closed without EOS` hazard is also plausible from the same split ordering: the all-closed thread can close another deferred stream before that stream has sent its own EOS response. - The producer-side trigger path is consistent with the issue. `VTabletWriterV2::on_partitions_created()` calls `_incremental_open_streams()`, which opens stream-map entries with `get_or_create(dst_id, true)`. `LoadStreamMap::get_or_create()` increments `_num_incremental_streams` only when a newly appearing backend stream entry is created, and `close_load()` sends that count in `PStreamHeader`. The auto-detect overwrite path calls FE `replacePartition`, updates the partition mapping, and invokes the same create-partition callback, so `INSERT OVERWRITE ... PARTITION(*)` can reach the incremental stream path. - Plain loads into already-opened streams, and named-partition overwrite that does not create runtime incremental streams, should not hit this specific receiver-side deferred-close race. - PR #65584 appears directionally correct for this root cause: it adds `mark_eos_sent_and_collect()`, registers a stream only after its own EOS response is sent, combines EOS-sent registration / all-close latch / collection under one lock, and adds a deterministic docker regression with a debug point. Missing information / validation: - Need CI/regression evidence for #65584, especially the new docker regression on current master. - If this fix is intended for 4.0.x, request branch-4.0 validation or a branch-4.0-specific backport patch. Current branch-4.0 has the same receiver-side race in the older `be/src/runtime/load_stream.cpp` path. - For any production occurrence, ask for BE logs around the affected load id showing `received CLOSE_LOAD`, close-wait behavior, and any sender-side `Stream closed without EOS` messages. These logs are not required to accept the root-cause mechanism, but they would help correlate real incidents. Recommended next steps: 1. Review PR #65584 as the likely fix for this issue. 2. Add the appropriate branch-pick label(s) if maintainers confirm the 4.0/4.0.x support scope. 3. In review, focus on the invariant that a stream must only be closed after its own EOS response is sent, and once all `CLOSE_LOAD`s are counted, every later EOS-sent registration must also drain and close. 4. Keep the issue open until the new regression passes on master and the backport target decision is made. -- 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]
