Doris-Breakwater commented on issue #69866: URL: https://github.com/apache/doris/issues/69866#issuecomment-6503349119
The report is supported by source inspection: both paths can leave a permanent positive offset in these BE metrics. I checked the public `4.1.4` tag (commit `ad35a140c7fd0b842f18c23300bac581f7d04326`) and current master (`2a4191c1dc94e789e1725d6cb345f46c31a17272`). Suggested triage: cloud/BE metric-accounting bug; the issue currently has no labels. This was a read-only investigation; no Doris build or runtime reproduction was performed. Verified findings: - **Duplicate additions are charged repeatedly.** `add_unused_rowsets()` increments bytes for every input and count by the input length, whereas the map retains only one entry per rowset ID. Removal subtracts once per erased entry. For an unchanged rowset of size B, adding it twice and successfully removing it leaves a residual count of 1 and bytes of B. This also applies to duplicate IDs within one input batch. See [4.1.4 accounting](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/cloud/cloud_tablet.cpp#L678-L709) and [master accounting](https://github.com/apache/doris/blob/2a4191c1dc94e789e1725d6cb345f46c31a17272/be/src/cloud/cloud_tablet.cpp#L678-L710). - **Destruction does not balance remaining entries.** `~CloudTablet()` is defaulted in both revisions. The tablet manager's cache-value destructor erases the tablet-map entry, and periodic unused-rowset cleanup traverses that map. Once the tablet is destroyed with entries remaining, their metric contributions survive. Cache eviction and object destruction are separate events: outstanding references can delay destruction. See [tablet destructor](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/cloud/cloud_tablet.cpp#L151-L154), [cache ownership](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/cloud/cloud_tablet_mgr.cpp#L103-L181), and [vacuum traversal](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/cloud/cloud_tablet_mgr.cpp#L290-L328). - **The reproduction must release extra rowset references.** Removal skips entries whose `shared_ptr::use_count()` exceeds 1. Clear the caller's rowset pointer and input vectors before the removal that is expected to succeed; otherwise the test demonstrates legitimate retention instead of the residual accounting error. The proposed fix direction is appropriate: charge only successful insertions under `_gc_mutex`, record the bytes actually charged, debit that same amount on removal, and debit only remaining entries at tablet destruction. Use signed deltas. Keep teardown accounting separate from the normal cleanup routine, which can skip pinned entries and performs cache/delete-bitmap work. Preserve master's existing `_meta_lock` → `_gc_mutex` order. One semantic detail deserves an explicit test: map assignment is only effectively a no-op for the same pointer; a different `Rowset` object with the same ID replaces the stored pointer today. `try_emplace` would retain the first object. Confirm that this is the intended ownership behavior, because cleanup uses that object's reference count. Byte bookkeeping should not introduce an additional owning rowset reference that prevents cleanup. Recommended regression coverage, using deltas from the process-global bvar baseline: - Duplicate addition across calls and within one batch; after releasing extra references, removal restores both metrics to baseline. - A retained reader reference prevents removal; releasing it allows balanced removal. - Destruction with pending entries, including cache eviction followed by release of the final tablet owner; both metrics return to baseline. - Partial removal followed by destruction, and destruction after complete removal, to catch double subtraction. - Distinct objects sharing one rowset ID; if implementing captured-byte accounting, verify that removal/destruction subtract the original charge even if the metadata size changes. Remaining uncertainty: source inspection proves the accounting defects but does not establish which production caller re-adds an already queued ID, or how much observed growth comes from each path. No additional logs are needed to justify the focused accounting fix. To attribute a production incident, please provide the exact BE build SHA, a minimal workload, timestamped metric samples, and correlated add/remove/eviction logs with rowset IDs and reference counts. An SQL profile is not necessary for this validation. Finally, these counters track the local unused-rowset bookkeeping; they do not directly measure all reader-pinned rowsets or remote object-store reclamation. Even corrected counters should be documented with that scope. The inspected stale-rowset path can recycle cached data before adding entries to this map. See [stale-rowset cleanup](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/cloud/cloud_tablet.cpp#L580-L646). The evidence here establishes an observability defect, not a physical-storage leak or data-loss condition. Breakwater-GitHub-Analysis-Slot: slot_737d56b0a8df -- 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]
