github-actions[bot] commented on code in PR #67778:
URL: https://github.com/apache/doris/pull/67778#discussion_r3978389794
##########
be/src/load/channel/load_channel.cpp:
##########
@@ -307,6 +310,9 @@ bool LoadChannel::is_finished() {
Status LoadChannel::cancel() {
_cancelled.store(true);
std::lock_guard<std::mutex> l(_lock);
Review Comment:
[P1] Keep cancellation from becoming cached success
This publishes cancellation while a local `TabletsChannel::close()` can be
waiting on builder bitmap work under its channel lock. The wait then returns
`CANCELLED`, but local close only appends `tablet_errors`, drops the writer
from the commit set, and returns top-level OK without committing it. An
external `LoadChannelMgr::cancel()` has already cached the cancel reason before
reaching here; the in-flight EOS can subsequently call
`_finish_load_channel()`, whose same-key LRU insert replaces that reason with
null success. Empty-reason cancellation and timeout cleanup can likewise
install success after the skipped commit. A retry of a lost EOS response is
then acknowledged as successful even though cancellation won first and the
writer was not committed. Please make a cancelled channel ineligible for
`_finish_load_channel()` while preserving the first terminal state, and cover
the local manager EOS/cancel and timeout races with blocked bitmap work plus a
retried EOS.
--
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]
