Repository navigation
[feature](be) Count the changelog TTL cleanups in the BE metrics - #69868
Conversation
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
The changelog of a table is cleaned up by TTL on the BEs: a cumulative compaction of a changelog tablet drops the
rowsets whose commit TSO is at or before the TTL cutoff instead of merging them. Nothing in /metrics says whether
this cleanup runs, fails, or how much it reclaims. The bvars changelog_ttl_discovered, _submitted and _failed are
only visible on the brpc /vars page and count what the TTL scanner found and submitted, not what the compactions
did: a cleanup that fails after it was submitted counts nowhere, and the bytes removed are not counted at all. The
only changelog series in /metrics, compaction_task_state_total{type="changelog"}, is the number of running and
pending changelog compactions. So a cleanup that keeps failing, which lets the changelog grow without bound, cannot
be alerted on.
This adds three BE metrics, counted where a cleanup finishes:
- doris_be_changelog_ttl_cleanup_total{result="success"|"failed"}: the compactions that drop expired changelog
rowsets, counted as each one ends.
- doris_be_changelog_ttl_reclaimed_bytes_total: the total disk size of the expired rowsets the successful ones
dropped.
- doris_be_changelog_ttl_last_success_timestamp_seconds: when one last succeeded.
A compaction is a cleanup when one of its inputs is expired changelog: at or below the visible version its TTL
snapshot was taken at, with a commit TSO at or before the cutoff. That is what filter_changelog_ttl_rowsets leaves
out of the merge, and the two now share one predicate, Compaction::is_expired_changelog_rowset, which
pick_expired_changelog_rowset uses as well. The decision is made from the inputs rather than from what the merge
filtered, so that a cleanup failing before its merge counts as a failure. It is counted as the execution of the
compaction ends, in CumulativeCompaction::execute_compact and in CloudCumulativeCompaction::record_compaction_success
and record_compaction_failure; a compaction that does not get past its preparation has not started a cleanup. It
does not matter whether the TTL scanner or the compaction producer submitted the compaction: both submit
CHANGELOG_COMPACTION, and either one picks expired rowsets first. In cloud mode the BE that runs the compaction
counts it, so a query sums over the BEs. The dropped rowsets become stale rowsets, and their files are deleted
later, by the stale rowset sweep or the recycler; the bytes are counted when the rowsets leave the tablet. The bvars
stay as they are.
### Release note
New BE metrics doris_be_changelog_ttl_cleanup_total{result}, doris_be_changelog_ttl_reclaimed_bytes_total and
doris_be_changelog_ttl_last_success_timestamp_seconds.
### Check List (For Author)
- Test: Unit Test, Regression test, Manual test
- Unit Test: CumulativeCompactionTest.TestRecordChangelogTtlCleanup (what counts as a cleanup, and that the
bytes are those of the rowsets filter_changelog_ttl_rowsets drops),
CompactionMetricsTest.TestChangelogTtlCleanupCountedAsCompactionFinishes (through
CumulativeCompaction::execute_compact, success and failure) and
CloudCompactionTest.changelog_ttl_cleanup_counted_as_compaction_finishes; with the other compaction, changelog
TTL and DorisMetrics suites, 114 cases pass and one is skipped outside Release. With the success hooks
removed, the two cases that go through them fail.
- Regression test: changelog_p0/test_changelog_ttl_cleanup_metrics (nonConcurrent) on a local shared-nothing
cluster; not run in cloud mode
- Manual test: on that cluster the four series are there from 0; after the cleanup of the regression table,
success is 1 and the reclaimed bytes are 1112, the input_rowsets_total_size the BE logs for the changelog
compaction that filtered both rows. promtool check metrics on the /metrics of the BE reports no parsing
error; the only finding on the new series is "no help text", which every BE series gets.
- Behavior changed: No (new metrics)
- Does this need documentation: Yes (doris-website, with the other changelog and IVM monitoring docs)
Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
|
run buildall |
There was a problem hiding this comment.
Reviewed the exact PR head 5c30820 against base 6991e76 using the authoritative diff. Three verified issues are included inline.
The feature goal is clear and the metric registration/export wiring is consistent: the grouped success/failed counter series, reclaimed-bytes counter, and timestamp gauge are registered once and use the expected Prometheus names. The local and cloud compaction flows were traced through selection, TTL filtering, merge/modify, commit/abort, and status propagation. No data-version, delete-bitmap, persistence, protocol, configuration, static-initialization, or memory-ownership regression was found.
Concurrency review found two issues: local cumulative execution can lose the reacquire lock after preparation and return before recording a failed TTL cleanup; and the process-wide last-success gauge uses an unconditional atomic store, so an older timestamp can overwrite a newer one when successful cleanups interleave. The tests also leave global TTL metric values modified in the monolithic doris_be_test process, making later tests order-dependent.
Testing and observability checkpoints were inspected statically. The changed BE tests, sync-point cleanup, regression scraper, metric labels, and backend aggregation were reviewed, but builds, unit tests, regression tests, benchmarks, and runtime endpoint checks were not executed under the review contract. The PR has no user-provided focus items and no existing inline P0/P1 findings to carry forward.
This bounded review reached round 3. Delivery is verified as a non-converged review: the latest assigned subagent set did not all return NO_NEW_VALUABLE_FINDINGS after the timestamp finding, and some follow-up passes were interrupted by model-capacity errors. Main-agent verification covered the full changed-file list and all remaining candidates; the residual limit is subagent consensus and lack of runtime execution evidence.
Review did not converge after 3 rounds. This automated review cannot pass the review check.
At the three-round cap, convergence was not reached: the latest assigned subagent set did not all return NO_NEW_VALUABLE_FINDINGS after the timestamp race was discovered, and R1/R4 follow-up passes were interrupted by temporary model-capacity errors. The main agent independently verified all candidates and the full changed-file list. Runtime builds, tests, regression execution, and live metric endpoint checks remain unexecuted because this contract is static-only.
| SCOPED_ATTACH_TASK(_mem_tracker); | ||
|
|
||
| st = CompactionMixin::execute_compact(); | ||
| record_changelog_ttl_cleanup(st.ok()); |
There was a problem hiding this comment.
[P2] Record TTL cleanup failure when the cumulative lock is unavailable. After preparation has selected expired inputs, another task can hold this lock before execute reacquires it; the TRY_LOCK_FAILED return at lines 115-118 bypasses record_changelog_ttl_cleanup, so the failed cleanup counter undercounts exactly the execution failure this metric is meant to expose. Route this early return through the same failure recording path while keeping the no-expired-input no-op.
There was a problem hiding this comment.
Thanks. The window is real: the lock is released between prepare_compact and execute_compact, and a manual compaction through /api/compaction/run, a clone or a disk migration can take it in between. But a compaction that loses the lock there has not started its cleanup, and counting it would make the series disagree with itself and with cloud mode.
- It is the same condition
prepare_compactchecks, found later: both returnTRY_LOCK_FAILEDwith "The tablet is under cumulative compaction", and a compaction that meets it inprepare_compactis not counted. Counting it only in the execution would count the same contention or not depending on whether it began while the task was queued. A lock held for long is met in the preparation of the later rounds, and would not show either way. - Cloud mode has the same gate and does not count it: the compaction job is requested from the meta service at the head of the pool task (
_request_tablet_global_compaction_lock), and when it is refused,JOB_TABLET_BUSYfor example, the task returns beforeexecute_compact, sorecord_compaction_failureis not reached. In both modes, counting starts once the compaction holds the cumulative compaction of the tablet: the lock taken again in shared-nothing mode, the job in cloud mode. - When the holder is another cumulative compaction, the manual one for example, it picks the expired rowsets first and counts the cleanup itself as it ends. Counting the one that lost the lock as failed would report one cleanup as a success and a failure.
- The compaction that loses the lock sets
last_cumu_compaction_failure_time, and the TTL scanner submits the tablet again aftermin_compaction_failure_interval_ms; the expired rowsets are still there. It is a retry, not a cleanup that cannot make progress.
The description said "does not get past its preparation", which leaves out the lock taken again in the execution. I reworded that sentence to name it.
So I'm keeping it as it is.
| if (rowset->end_version() > _changelog_ttl_visible_version || | ||
| !changelog_rowset_expired(*rowset->rowset_meta(), _changelog_ttl_cutoff_tso)) { | ||
| if (!is_expired_changelog_rowset(rowset)) { | ||
| _data_input_rowsets.push_back(rowset); |
There was a problem hiding this comment.
[P2] Publish the maximum success timestamp. Two successful cleanups can interleave between UnixSeconds() and this unconditional set_value: an older timestamp can overwrite a newer one, making changelog_ttl_last_success_timestamp_seconds move backwards and trigger stale-cleanup alerts. Update with an atomic max/CAS loop or serialize publication so the gauge remains monotonic.
There was a problem hiding this comment.
Thanks. Two successful cleanups can interleave there, but what that costs is bounded, and a max would cost more.
- The window is the two steps of one expression: reading the clock and the atomic store.
UnixSeconds()has a resolution of one second, so two threads read the same or adjacent seconds, and the gauge can go back by about a second until the next success sets it again. A stale-cleanup alert,time() - doris_be_changelog_ttl_last_success_timestamp_seconds > threshold, needs a threshold of several rounds of scheduling and compaction; a second does not change it. - The other last-time series of the BE that many threads publish,
fragment_last_active_timeandfile_cache_warm_up_rowset_last_finish_unix_ts, are stored the same way, and so isset_to_current_time()in the Prometheus clients. - A max would pin the gauge to the future once a cleanup succeeds while the clock is ahead, a clock set a day forward and then corrected for example:
time() - gaugestays negative until real time catches up, and no stale-cleanup alert can fire in the meantime, even with every cleanup stopped. That is worse than a second of jitter. Serializing the publication avoids that, but it adds a lock for a second that no alert can observe, which the other series do without.
So I'm keeping it as it is.
| for (const auto& rowset : compaction._input_rowsets) { | ||
| if (rowset->num_rows() != 0 && | ||
| std::find(merged.begin(), merged.end(), rowset) == merged.end()) { | ||
| dropped_bytes += rowset->total_disk_size(); |
There was a problem hiding this comment.
[P2] Restore the global TTL metrics after this test. DorisMetrics::instance() is shared by the monolithic doris_be_test binary, but this test leaves the timestamp gauge at 1 and the cloud counterpart leaves all four new values incremented. Snapshot and restore the new counters/gauge with Defer so later tests do not depend on test order.
There was a problem hiding this comment.
Thanks. They do stay changed, by CompactionMetricsTest.TestChangelogTtlCleanupCountedAsCompactionFinishes too, but no test depends on them, so the order does not matter.
- The only readers of these four series in
be/testare the three new tests, and each compares against values it takes right before: the counters by their difference, the timestamp against the time the test started after a success, and against the value it set or read itself after a failure.DorisMetricsTest.Normaldoes not read them. - What another suite reads as an absolute value is restored: recording a cloud success also adds to
cumulative_compaction_deltas_totalandcumulative_compaction_bytes_total, whichDorisMetricsTest.Normalcompares as absolute values in the same process, so the cloud test restores those two with aDefer(see the comment there). - A test that reads these series later should compare differences too, rather than count on the tests before it to restore them.
So I'm keeping it as it is.
TPC-H: Total hot run time: 28559 ms |
TPC-DS: Total hot run time: 151472 ms |
ClickBench: Total hot run time: 24.33 s |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
|
PR approved by anyone and no changes requested. |
|
PR approved by at least one committer and no changes requested. |
What problem does this PR solve?
Issue Number: #69907
Related PR: None
Problem Summary:
In short. The BEs clean up the changelog of a table by TTL, but
/metricscannot tell whether that cleanup runs, fails, or reclaims anything. This PR counts each cleanup on the BE as it finishes, in three new series: how many succeeded or failed, how many bytes of expired changelog they dropped, and when one last succeeded.Background. A table with
changelog.enablekeeps its changelog in a hidden changelog index, andchangelog.ttl_secondsbounds how long the changelog is kept. The BEs remove what is older than that in a cumulative compaction of a tablet of that index (a changelog tablet): rowsets whose commit TSO is at or before the TTL cutoff are left out of the merge, so the output replaces them without their rows. Two submitters start these compactions, both asCHANGELOG_COMPACTION: the changelog TTL scanner, for tablets it finds holding expired rowsets, and the regular compaction producer; either compaction picks expired rowsets first.The problem, and what it cost.
changelog_ttl_discovered,changelog_ttl_submittedandchangelog_ttl_failedare only on the brpc/varspage, and they count what the TTL scanner found and submitted, not what the compactions did: a cleanup that fails after it was submitted counts nowhere, and nothing counts the bytes removed./metrics,compaction_task_state_total{type="changelog"}, is the number of changelog compactions running and pending. A cleanup that keeps failing lets the changelog grow without bound, and nothing on the BE says why.How this PR fixes it.
doris_be_changelog_ttl_cleanup_totalresult=success/faileddoris_be_changelog_ttl_reclaimed_bytes_totaldoris_be_changelog_ttl_last_success_timestamp_secondsfilter_changelog_ttl_rowsetsleaves out of the merge, and the two now ask one predicate,Compaction::is_expired_changelog_rowset, whichpick_expired_changelog_rowsetuses too. It is decided from the inputs rather than from what the merge filtered, so that a cleanup that fails before its merge, a cloud compaction that cannot refresh its tablet meta for example, is counted as failed.CumulativeCompaction::execute_compactin shared-nothing mode,CloudCumulativeCompaction::record_compaction_successandrecord_compaction_failurein cloud mode. A compaction that does not get to run has not started a cleanup and is not counted: one that finds the tablet under another cumulative compaction, whether in its preparation or as its execution takes the cumulative compaction lock again, and one whose job the meta service refuses.Results. Before this PR, the
/metricsof a BE has none of these series. After it, on a local shared-nothing cluster, a table withchangelog.ttl_seconds= 1 and two rows inserted (regressiontest_changelog_ttl_cleanup_metrics):cleanup_total{result="success"}cleanup_total{result="failed"}reclaimed_bytes_totallast_success_timestamp_secondsThe 1112 bytes are what the BE logs for that changelog compaction:
input_rowsets_total_size=1.09 KB,output_rowset_total_size=0,filtered_row_num=2.Classes, and how they call each other.
DorisMetrics: the three series.Compaction:is_expired_changelog_rowset(new), shared bypick_expired_changelog_rowsetandfilter_changelog_ttl_rowsets;record_changelog_ttl_cleanup(new) counts a cleanup from the inputs.CumulativeCompaction,CloudCumulativeCompaction: call it as the execution ends.Nothing else changes: no config, no thrift or proto change, and what the compactions pick and drop stays the same.
Release note
New BE metrics
doris_be_changelog_ttl_cleanup_total{result},doris_be_changelog_ttl_reclaimed_bytes_totalanddoris_be_changelog_ttl_last_success_timestamp_seconds.Check List (For Author)
Test
Unit test:
CumulativeCompactionTest.TestRecordChangelogTtlCleanupchecks what counts as a cleanup (not a compaction whose inputs hold no expired changelog, nor one that took no TTL snapshot; a rowset above the visible version of the snapshot, or an empty one, is not expired changelog) and that the reclaimed bytes are those of the rowsetsfilter_changelog_ttl_rowsetsdrops;CompactionMetricsTest.TestChangelogTtlCleanupCountedAsCompactionFinishesrunsCumulativeCompaction::execute_compactup to its merge and checks a success and a failure;CloudCompactionTest.changelog_ttl_cleanup_counted_as_compaction_finishesdoes the same throughrecord_compaction_successandrecord_compaction_failure. With the success hooks removed, the two that go through them fail. Together with the other compaction, changelog TTL andDorisMetricssuites, 114 cases pass and one is skipped outside Release.Regression test:
changelog_p0/test_changelog_ttl_cleanup_metrics(nonConcurrent: it waits for the TTL to expire), on a local shared-nothing cluster; not run in cloud mode.Manual test:
promtool check metricson the/metricsof the BE reports no parsing error; the only finding on the new series is "no help text", which every BE series gets (the BE writes no HELP lines).Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)