Skip to content

gc: expose per-pdev backlog and pressure metrics - #462

Merged
xiaoxichen merged 1 commit into
eBay:stable/v4.xfrom
xiaoxichen:gc_metrics
Sep 3, 2026
Merged

gc: expose per-pdev backlog and pressure metrics#462
xiaoxichen merged 1 commit into
eBay:stable/v4.xfrom
xiaoxichen:gc_metrics

Conversation

@xiaoxichen

Copy link
Copy Markdown
Collaborator

Add five per-pdev gauges to pdev_gc_metrics for operator visibility into the GC backlog:

  • pending_gc_bytes reclaimable bytes in PG-owned chunks
  • eligible_gc_bytes bytes GC would pick up under current policy
  • eligible_gc_chunk_count count for the above
  • pending_normal_gc_task_count normal-priority queue depth
  • pending_gc_chunks_ratio 10-series labeled gauge over (0,10]% .. (90,100]%

The bucket metric emits as a single Prometheus family with a 'bucket' label, differentiated by direct m_impl_ptr->register_gauge() calls (the compile-time-name macro cannot express label-differentiated series). Descriptions embed the bucket label so the sisl JSON dump does not collapse the 10 entries to one.

Values are refreshed once per gc_scan_interval by scan_chunks_for_gc, using a new private GCManager::get_chunk_gc_snapshot helper that folds the four ExtendedVChunk field reads into a single lookup per chunk; get_chunk_gc_ratio is retained as a thin wrapper for backward compatibility.

@codecov-commenter

codecov-commenter commented Aug 28, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 83.09859% with 12 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (stable/v4.x@f933d1d). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/lib/homestore_backend/gc_manager.cpp 72.09% 8 Missing and 4 partials ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@              Coverage Diff               @@
##             stable/v4.x     #462   +/-   ##
==============================================
  Coverage               ?   53.99%           
==============================================
  Files                  ?       39           
  Lines                  ?     6889           
  Branches               ?      943           
==============================================
  Hits                   ?     3720           
  Misses                 ?     2767           
  Partials               ?      402           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JacksonYao287 JacksonYao287 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally , LGTM. better to wait for @Besroy and @yuwmao to also take a look.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when reaching here, it means some gc tasks are backloged. it is valuable for us to report eligible_gc_bytes/eligible_gc_chunk_count/pending_gc_chunks_ratio , etc. to metrics now to show us the gc pressure.

if we just continue and skip publish_scan_snapshot, we can not get those information timely.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense, we should remove this checks to ensure we refreshed the metrics

Add five per-pdev gauges to pdev_gc_metrics for operator visibility into
the GC backlog:

  - pending_gc_bytes            reclaimable bytes in PG-owned chunks
  - eligible_gc_bytes           bytes GC would pick up under current policy
  - eligible_gc_chunk_count     count for the above
  - pending_normal_gc_task_count normal-priority queue depth
  - pending_gc_chunks_ratio     10-series labeled gauge over (0,10]% .. (90,100]%

The bucket metric emits as a single Prometheus family with a 'bucket'
label, differentiated by direct m_impl_ptr->register_gauge() calls (the
compile-time-name macro cannot express label-differentiated series).
Descriptions embed the bucket label so the sisl JSON dump does not
collapse the 10 entries to one.

Values are refreshed once per gc_scan_interval by scan_chunks_for_gc,
using a new private GCManager::get_chunk_gc_snapshot helper that folds
the four ExtendedVChunk field reads into a single lookup per chunk;
get_chunk_gc_ratio is retained as a thin wrapper for backward
compatibility.

Metric accumulation and publish_scan_snapshot run unconditionally for
every pdev; only the submission phase is gated by the saturation
check. Freezing the backlog gauges when the normal-GC queue is
saturated would blind operators to exactly the pressure these metrics
are meant to surface.
@Besroy

Besroy commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Generally LGTM. Just curious why we want to add these metrics --- eligible makes sense to me, but could you share more context on why we track pending cases where there is only a small amount of garbage and the threshold has not been reached yet?

BTW from my experience during triage, metrics like “running GC task count” or “GC task cost time” may be more helpful for identifying stuck GC tasks.

@xiaoxichen

Copy link
Copy Markdown
Collaborator Author

Generally LGTM. Just curious why we want to add these metrics --- eligible makes sense to me, but could you share more context on why we track pending cases where there is only a small amount of garbage and the threshold has not been reached yet?

BTW from my experience during triage, metrics like “running GC task count” or “GC task cost time” may be more helpful for identifying stuck GC tasks.

we are seeing a significant amount of capacity drift between "used capacity report by SM-> CM->DM" vs "user consumed capacity reported by usage exporter" so the added metrics is to capture how much space we can reclaim in theory vs how much spac our configuration allow us to reclaim.

@xiaoxichen
xiaoxichen merged commit 1613539 into eBay:stable/v4.x Sep 3, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants