github-actions[bot] commented on code in PR #65651:
URL: https://github.com/apache/doris/pull/65651#discussion_r3587612450
##########
be/src/load/stream_load/stream_load_recorder_manager.cpp:
##########
@@ -242,7 +242,8 @@ void StreamLoadRecorderManager::_load_if_necessary() {
Status st = _send_stream_load(_buffer.ToString());
if (!st.ok()) {
LOG(WARNING) << "Failed to load stream load records to audit log
table: " << st
- << ", discard current batch";
+ << ", keep current batch for retry";
+ return;
Review Comment:
Keeping the batch for retry fixes the cursor-loss case, but this path still
needs a bounded and idempotent retry state. The worker always fetches before
`_load_if_necessary()`, so after this `return` every failed attempt leaves
`_last_load_time` and `_buffer` unchanged, then the next tick appends another
`get_batch()` worth of records before retrying the whole payload.
`stream_load_record_batch_bytes` is only a send trigger, not a cap, so an
audit-load outage can make the BE retain, copy, and upload an ever-growing
buffer.
There is also an unknown-outcome case: `_send_stream_load()` creates a new
label for every retry and treats non-`Success` responses such as `Publish
Timeout` as failure, while the stream-load path does not roll back publish
timeouts. Retrying the same rows under a fresh label can append duplicate audit
rows. Please make the retained batch a bounded, idempotent retry unit, for
example by not fetching past a pending batch and by keeping a stable per-batch
label / handling already-accepted outcomes before returning here.
--
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]
