github-actions[bot] commented on code in PR #63295:
URL: https://github.com/apache/doris/pull/63295#discussion_r3317782929


##########
cloud/src/recycler/recycler.cpp:
##########
@@ -5152,18 +5208,123 @@ int InstanceRecycler::recycle_rowsets() {
                 LOG(WARNING) << "failed to delete recycle rowset kv, 
instance_id=" << instance_id_;
                 return;
             }
+
             num_recycled.fetch_add(rowset_keys_to_delete.size(), 
std::memory_order_relaxed);
         });
+    };
+
+    auto submit_mark_abort_rowset_job = [&](std::vector<std::string> 
rowset_keys_to_mark,
+                                            std::vector<std::string> 
rowset_keys_to_abort) {
+        if (rowset_keys_to_mark.empty() && rowset_keys_to_abort.empty()) {
+            return;
+        }
+        mark_abort_worker_pool->submit([&, rowset_keys_to_mark = 
std::move(rowset_keys_to_mark),
+                                        rowset_keys_to_abort =
+                                                
std::move(rowset_keys_to_abort)]() mutable {
+            auto start = steady_clock::now();
+            DORIS_CLOUD_DEFER {
+                auto cost = duration_cast<milliseconds>(steady_clock::now() - 
start).count();
+                LOG(INFO) << "finish mark and abort rowset job, instance_id=" 
<< instance_id_ << ' '
+                          << "cost_ms=" << cost << ' '
+                          << "rowset_keys_to_mark.size()=" << 
rowset_keys_to_mark.size() << ' '
+                          << "rowset_keys_to_abort.size()=" << 
rowset_keys_to_abort.size();
+            };
+            if (!rowset_keys_to_mark.empty() &&
+                batch_mark_rowsets_as_recycled<RecycleRowsetPB>(txn_kv_.get(), 
instance_id_,
+                                                                
rowset_keys_to_mark) != 0) {
+                LOG(WARNING) << "failed to batch mark rowsets as recycled, 
instance_id="
+                             << instance_id_ << ' '
+                             << "rowset_keys_to_mark.size()=" << 
rowset_keys_to_mark.size();
+                return;
+            }
+            if (!rowset_keys_to_abort.empty() &&
+                
batch_abort_txn_or_job_for_recycle<RecycleRowsetPB>(rowset_keys_to_abort, true) 
!=
+                        0) {
+                LOG(WARNING) << "failed to batch abort txn or job for related 
rowset, "
+                                "instance_id="
+                             << instance_id_ << ' '
+                             << "rowset_keys_to_abort.size()=" << 
rowset_keys_to_abort.size();
+                return;
+            }
+        });

Review Comment:
   This flush gate only considers rowsets that have data, so metadata-only 
entries in `rowset_keys_without_data` can grow without bound until the entire 
scan finishes. For example, a backlog of expired empty rowsets (`num_segments() 
== 0`) or old-version rowsets with empty `resource_id` keeps `rowsets.size() == 
0`, so every `loop_done()` during the scan returns here and no KV-removal jobs 
are submitted until the final post-scan call. The old code swapped and 
submitted `rowset_keys` on each scan batch, while this defeats the new 
batching/memory-control intent and can retain all such keys for a large recycle 
backlog. Please include `rowset_keys_without_data` in the flush threshold, or 
flush it independently before returning.



-- 
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]

Reply via email to