github-actions[bot] commented on code in PR #66847:
URL: https://github.com/apache/doris/pull/66847#discussion_r3804798961
##########
be/src/cloud/cloud_delta_writer.cpp:
##########
@@ -99,9 +101,30 @@ Status CloudDeltaWriter::write(const Block* block, const
TabletAddRowsPayload& r
CHECK(_is_init || _is_cancelled);
{
SCOPED_TIMER(_wait_flush_limit_timer);
- while (_memtable_writer->flush_running_count() >=
- config::memtable_flush_running_count_limit) {
+ const auto effective_flush_running_count_limit =
+ config::memtable_flush_running_count_limit *
+ (_req.write_req_type == WriteRequestType::GROUP ? 2 : 1);
+ const bool use_s3_upload_queue = rowset_builder()->is_s3_storage();
Review Comment:
[P1] Return cancellation before probing the rowset writer. A cancel can win
after _get_current_seq() releases the channel lock but before batch_init()
acquires this writer: cancel marks the still-uninitialized MemTableWriter
cancelled, batch_init() deliberately skips it, and the in-flight add-batch
still reaches write(). The CHECK above explicitly accepts that _is_cancelled &&
!_is_init state, but is_s3_storage() then dereferences
CloudRowsetBuilder::_rowset_writer, which was never created, so a normal
timeout/cancel race can crash the BE instead of returning the saved
cancellation status. Before this change, the null flush token returned a count
of zero and MemTableWriter::write() propagated cancellation. Please
short-circuit the known cancelled state before any rowset/filesystem access
(and assert initialization for the probe), with a synchronized
cancel-before-batch-init test.
##########
be/src/load/delta_writer/delta_writer.cpp:
##########
@@ -178,8 +178,10 @@ Status DeltaWriter::write(const Block* block, const
TabletAddRowsPayload& rows,
}
{
SCOPED_TIMER(_wait_flush_limit_timer);
- while (_memtable_writer->flush_running_count() >=
- config::memtable_flush_running_count_limit) {
+ const auto effective_flush_running_count_limit =
+ config::memtable_flush_running_count_limit *
+ (_req.write_req_type == WriteRequestType::GROUP ? 2 : 1);
Review Comment:
[P1] Count logical GROUP flushes before they enter the shared pool. Each
full GROUP memtable submits separate DATA and ROW_BINLOG tasks, but
flush_running_count increments only after a worker dequeues a half. With limit
2 and a two-worker pool (or the adaptive controller shrinking a four-core BE to
its two-thread minimum), M1's pair runs with count 2; this new threshold is 4,
so M2, M3, and later pairs keep entering the unbounded pool queue while their
SharedMemtables remain retained and invisible to this check. The old threshold
2 stopped the writer before M2, so this newly removes backpressure for a
reachable local/HDFS schedule. Please reserve/count one logical pending
memtable before submitting the pair and release it only after both halves
finish on every success/error/cancel path, with a stalled two-worker
global/workload-group test.
##########
be/src/load/delta_writer/delta_writer.cpp:
##########
@@ -178,8 +178,10 @@ Status DeltaWriter::write(const Block* block, const
TabletAddRowsPayload& rows,
}
{
SCOPED_TIMER(_wait_flush_limit_timer);
- while (_memtable_writer->flush_running_count() >=
- config::memtable_flush_running_count_limit) {
+ const auto effective_flush_running_count_limit =
Review Comment:
[P2] Keep the mutable flush limit live while waiting.
memtable_flush_running_count_limit is registered with DEFINE_mInt32, and the
old condition re-read it on every 10 ms poll. Capturing it once per write means
that if a local or HDFS writer blocks at 2 and an operator raises the limit to
4, that in-flight RPC continues comparing against 2 until an old flush
finishes; with a stalled flush, the runtime change can no longer release it.
The cloud non-S3/fallback branch has the same snapshot. Please recompute the
effective GROUP/non-GROUP limit on each loop iteration, and cover changing the
limit while a writer is waiting.
--
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]
