github-actions[bot] commented on code in PR #65550:
URL: https://github.com/apache/doris/pull/65550#discussion_r3620238810
##########
cloud/src/recycler/recycler.cpp:
##########
@@ -5178,6 +5096,36 @@ int InstanceRecycler::recycle_rowsets() {
<< " creation_time=" << rowset_meta->creation_time()
<< " task_type=" << metrics_context.operation_type;
if (rowset.type() == RecycleRowsetPB::PREPARE) {
+ if (config::enable_mark_delete_rowset_before_recycle) {
+ if (need_mark_rowset_as_recycled(rowset.rowset_meta())) {
+ rowset_keys_to_mark_recycled.emplace_back(k);
+ LOG(INFO) << "rowset queued to mark as recycled, recycler
will delete data and "
+ "kv "
+ "at next turn, instance_id="
+ << instance_id_ << " tablet_id=" <<
rowset_meta->tablet_id()
+ << " version=[" << rowset_meta->start_version()
<< '-'
+ << rowset_meta->end_version() << "]";
+ return 0;
+ }
+ }
+
+ if
(config::enable_abort_txn_and_job_for_delete_rowset_before_recycle &&
+ rowset_meta->end_version() != 1) {
+ int ret = 0;
+ if (rowset_meta->has_load_id()) {
+ DCHECK(rowset_meta->has_txn_id() && rowset_meta->txn_id()
> 0);
+ ret = abort_txn_for_related_rowset(rowset_meta->txn_id());
Review Comment:
[P2] Keep PREPARE aborts bounded-concurrent
`scan_and_recycle()` executes this branch serially, and each txn/job abort
performs several metadata reads plus a commit before the next key is scanned.
With the default 1,000-rowset per-tablet limit and no aggregate PREPARE cap, a
many-tablet expired backlog can serialize hundreds of thousands of round-trip
sequences on one scan thread; the removed dedicated pool previously overlapped
page-sized abort batches. Please preserve the abort-before-delete barrier with
bounded concurrent abort work, wait for the results, and submit deletion only
for successfully aborted keys.
##########
cloud/src/recycler/recycler.cpp:
##########
@@ -5178,6 +5096,36 @@ int InstanceRecycler::recycle_rowsets() {
<< " creation_time=" << rowset_meta->creation_time()
<< " task_type=" << metrics_context.operation_type;
if (rowset.type() == RecycleRowsetPB::PREPARE) {
+ if (config::enable_mark_delete_rowset_before_recycle) {
+ if (need_mark_rowset_as_recycled(rowset.rowset_meta())) {
+ rowset_keys_to_mark_recycled.emplace_back(k);
+ LOG(INFO) << "rowset queued to mark as recycled, recycler
will delete data and "
+ "kv "
+ "at next turn, instance_id="
+ << instance_id_ << " tablet_id=" <<
rowset_meta->tablet_id()
+ << " version=[" << rowset_meta->start_version()
<< '-'
+ << rowset_meta->end_version() << "]";
+ return 0;
+ }
+ }
+
+ if
(config::enable_abort_txn_and_job_for_delete_rowset_before_recycle &&
+ rowset_meta->end_version() != 1) {
+ int ret = 0;
+ if (rowset_meta->has_load_id()) {
+ DCHECK(rowset_meta->has_txn_id() && rowset_meta->txn_id()
> 0);
+ ret = abort_txn_for_related_rowset(rowset_meta->txn_id());
+ } else if (rowset_meta->has_job_id()) {
+ ret = abort_job_for_related_rowset(*rowset_meta);
+ }
+ if (ret != 0) {
Review Comment:
[P1] Allow recycler ABORT to remove expired jobs
The recycler reaches a PREPARE key only after its persisted expiration plus
retention; compaction output copies the job deadline into that expiration, so
the matching recorded job is already expired in this path.
`abort_job_for_related_rowset()` calls `_finish_tablet_job()`, where
`process_compaction_job()` returns `JOB_EXPIRED` before its ABORT branch
(schema change has the same ordering). This new failure gate therefore returns
on every pass and leaves both the object data and recycle key indefinitely on
an idle tablet. Please make recycler-initiated ABORT remove the exact expired
job transactionally before deletion, and cover the expired-job case.
##########
cloud/src/recycler/recycler.cpp:
##########
@@ -5178,6 +5096,36 @@ int InstanceRecycler::recycle_rowsets() {
<< " creation_time=" << rowset_meta->creation_time()
<< " task_type=" << metrics_context.operation_type;
if (rowset.type() == RecycleRowsetPB::PREPARE) {
+ if (config::enable_mark_delete_rowset_before_recycle) {
+ if (need_mark_rowset_as_recycled(rowset.rowset_meta())) {
+ rowset_keys_to_mark_recycled.emplace_back(k);
+ LOG(INFO) << "rowset queued to mark as recycled, recycler
will delete data and "
+ "kv "
+ "at next turn, instance_id="
+ << instance_id_ << " tablet_id=" <<
rowset_meta->tablet_id()
+ << " version=[" << rowset_meta->start_version()
<< '-'
+ << rowset_meta->end_version() << "]";
+ return 0;
+ }
+ }
+
+ if
(config::enable_abort_txn_and_job_for_delete_rowset_before_recycle &&
+ rowset_meta->end_version() != 1) {
+ int ret = 0;
+ if (rowset_meta->has_load_id()) {
+ DCHECK(rowset_meta->has_txn_id() && rowset_meta->txn_id()
> 0);
+ ret = abort_txn_for_related_rowset(rowset_meta->txn_id());
+ } else if (rowset_meta->has_job_id()) {
+ ret = abort_job_for_related_rowset(*rowset_meta);
Review Comment:
[P1] Abort the job named by this PREPARE rowset
`abort_job_for_related_rowset()` finds the matching ID anywhere in
`job_pb.compaction()` but then copies the whole record; `_finish_tablet_job()`
always processes `request.job().compaction(0)`. With supported parallel
non-overlapping compactions, a target later in the repeated field makes the
recycler abort an unrelated job, report success, and delete the target output
while its owner remains live. If schema change coexists with compactions, the
helper's `else if` never searches it either. Build the ABORT request from only
the exact matched job (plus the tablet index), and verify that exact entry was
removed before allowing deletion.
--
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]
