github-actions[bot] commented on code in PR #66919:
URL: https://github.com/apache/doris/pull/66919#discussion_r3849110050
##########
cloud/src/recycler/recycler.cpp:
##########
@@ -4807,6 +4815,14 @@ int InstanceRecycler::recycle_tablet(int64_t tablet_id,
RecyclerMetricsContext&
.tag("rowset_id", rs_meta.rowset_id_v2());
return -1;
}
Review Comment:
[P1] Decrement every packed DBM before deleting the tablet range
These loops cover current and restore-job rowsets only, but the bulk removal
below also destroys DBMs whose only rowset owner is elsewhere. A compacted
rowset can exist only in `RecycleRowsetPB`, and a load writes `meta_rowset_tmp`
before saving its packed DBM; tmp keys are transaction-first and may remain for
72 hours after an index becomes recyclable. If tablet recycling wins, it
removes those owners/DBM locations without decrementing their packed slices. A
slice already marked `corrected` is never rechecked, so its `data/packed_file`
object and KV stay live indefinitely. Please enumerate the tablet's DBM range
itself (or otherwise prove every owner namespace is drained) before removing
it, and test packed DBMs owned by recycled and uncommitted tmp rowsets.
##########
cloud/src/recycler/recycler.cpp:
##########
@@ -4755,6 +4755,14 @@ int InstanceRecycler::recycle_tablet(int64_t tablet_id,
RecyclerMetricsContext&
.tag("rowset_id", rs_meta.rowset_id_v2());
return -1;
}
+ if (decrement_delete_bitmap_packed_file_ref_counts(tablet_id,
rs_meta.rowset_id_v2(),
Review Comment:
[P1] Read all owner sets before the per-rowset work
`txn` first reads the formal rowsets, then stays open while this helper
creates a separate transaction and performs a `blob_get` for every rowset—even
when no delete bitmap exists. Only after all of that sequential work is the
original snapshot reused by `scan_restore_job_rowset()`. With the configured
10-second transaction timeout and FoundationDB's read-version lifetime, a large
tablet can repeatedly fail that scan with `TXN_TIMEOUT` or `TXN_TOO_OLD`.
Please collect the restore-job rows from the same snapshot before doing any
per-rowset DBM work (or otherwise preserve the snapshot while refreshing), and
cover the stale-transaction path with fault injection.
--
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]
