Skip to content

gc: priority scheduling with dual watermarks and cross-scan quota#436

Open
xiaoxichen wants to merge 3 commits into
eBay:stable/v4.xfrom
xiaoxichen:gc-sort
Open

gc: priority scheduling with dual watermarks and cross-scan quota#436
xiaoxichen wants to merge 3 commits into
eBay:stable/v4.xfrom
xiaoxichen:gc-sort

Conversation

@xiaoxichen

Copy link
Copy Markdown
Collaborator
  • Sort eligible chunks by garbage ratio (desc) before submission so the most garbage-heavy chunks are always GC'd first
  • Add gc_garbage_rate_threshold_low (default 30%) as a low watermark; chunks between the two watermarks consume at most half the quota
  • Track m_pending_normal_gc_task_count in pdev_gc_actor to reflect tasks queued or running in m_gc_executor across scan cycles; previous code only capped submissions per scan, allowing unbounded queue growth
  • scan_chunks_for_gc now skips a pdev entirely when already at quota, and derives low_tier_cap proportionally from remaining_capacity
  • Add ADR docs/adr/gc-priority-scheduling.md

- Sort eligible chunks by garbage ratio (desc) before submission so the
  most garbage-heavy chunks are always GC'd first
- Add gc_garbage_rate_threshold_low (default 30%) as a low watermark;
  chunks between the two watermarks consume at most half the quota
- Track m_pending_normal_gc_task_count in pdev_gc_actor to reflect tasks
  queued or running in m_gc_executor across scan cycles; previous code
  only capped submissions per scan, allowing unbounded queue growth
- scan_chunks_for_gc now skips a pdev entirely when already at quota,
  and derives low_tier_cap proportionally from remaining_capacity
- Add ADR docs/adr/gc-priority-scheduling.md

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Xiaoxi Chen <xiaoxchen@ebay.com>
@codecov-commenter

codecov-commenter commented Jun 17, 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 66.00000% with 17 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (stable/v4.x@e1c23e1). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/lib/homestore_backend/gc_manager.cpp 64.58% 13 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     #436   +/-   ##
==============================================
  Coverage               ?   54.23%           
==============================================
  Files                  ?       36           
  Lines                  ?     5423           
  Branches               ?      686           
==============================================
  Hits                   ?     2941           
  Misses                 ?     2175           
  Partials               ?      307           

☔ 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.

Comment thread src/lib/homestore_backend/gc_manager.cpp Outdated
Comment thread src/lib/homestore_backend/gc_manager.cpp
eligible.push_back({chunk_id, ratio_pct});
}

// Sort eligible chunks by garbage ratio descending so the most garbage-heavy chunks are GC'd first.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is a topK problem.
suggest to use a heap with a capacity of remaining_capacity (priority_queue), so that we can hold at most remaining_capacity ChunkGCInfo in memory. and for any new ChunkGCInfo, we only need to compare it with the top of the heap.

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.

IMO It doesnt worth those lines of code , considering the maximum chunk is 32K.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

better to have it since it is not complicated and will also make code more simple, for example, the loop on the entire chunk collection will not be involved.

Comment thread src/lib/homestore_backend/gc_manager.cpp
Signed-off-by: Xiaoxi Chen <xiaoxchen@ebay.com>
Comment thread src/lib/homestore_backend/gc_manager.cpp
Comment thread src/lib/homestore_backend/gc_manager.cpp Outdated
- scan_chunks_for_gc: replace std::vector + std::sort with a bounded
  std::priority_queue (min-heap) of capacity max_task_num. Memory is now
  O(K) per pdev regardless of chunk count. K is fixed at max_task_num so
  we always retain enough candidates if capacity opens up later in the
  scan; submission is still gated by the dynamic remaining_capacity /
  low_tier_cap.

- add_gc_task: move m_pending_normal_gc_task_count.fetch_add(1) BEFORE
  m_gc_executor->add() so an immediately-completing task (which
  decrements via ~gc_task_guard) cannot underflow the counter.

- process_gc_task: when we early-return because the chunk is not in GC
  state, gc_task_guard is never constructed, so its decrement never
  fires. Decrement m_pending_normal_gc_task_count for normal-priority
  tasks here to mirror the guard's bookkeeping and prevent a permanent
  quota leak.

- docs/adr: update ADR to describe top-K heap selection instead of full
  sort.
@xiaoxichen

Copy link
Copy Markdown
Collaborator Author

@JacksonYao287 addressed

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.

3 participants