Skip to content

feat: new api for compaction#429

Open
dkharms wants to merge 1 commit into
mainfrom
0-fm-compaction-api
Open

feat: new api for compaction#429
dkharms wants to merge 1 commit into
mainfrom
0-fm-compaction-api

Conversation

@dkharms

@dkharms dkharms commented May 22, 2026

Copy link
Copy Markdown
Member

Description

In this PR I've implemented all necessary API for compaction. And also changed visibility of all methods inside fracmanager package since they should not be exported.

I expect that there will be slight tweaks and changes to this API (e.g. method signature change) but the whole idea will be the same.


  • I have read and followed all requirements in CONTRIBUTING.md;
  • I used LLM/AI assistance to make this pull request;

If you have used LLM/AI assistance please provide model name and full prompt:

Model: {{model-name}}
Prompt: {{prompt}}

@dkharms dkharms force-pushed the 0-fm-compaction-api branch from 1f7bfdb to 96ba81f Compare May 22, 2026 11:26
@dkharms dkharms marked this pull request as ready for review May 22, 2026 11:26
@eguguchkin eguguchkin requested review from cheb0 and eguguchkin May 25, 2026 11:26
@eguguchkin eguguchkin force-pushed the 275-fractions-snapshot branch from e447d7f to 8335b37 Compare June 2, 2026 17:07
@eguguchkin eguguchkin added this to the v0.73.0 milestone Jun 2, 2026
@eguguchkin eguguchkin force-pushed the 275-fractions-snapshot branch from b3f54cd to 8dd3663 Compare June 8, 2026 20:45
Base automatically changed from 275-fractions-snapshot to main June 8, 2026 21:26
@dkharms dkharms force-pushed the 0-fm-compaction-api branch from 96ba81f to d657428 Compare June 10, 2026 09:48
@codecov-commenter

codecov-commenter commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 52.81690% with 67 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.60%. Comparing base (9b461f4) to head (b6e78ca).

Files with missing lines Patch % Lines
fracmanager/fraction_registry.go 53.06% 44 Missing and 2 partials ⚠️
fracmanager/fracmanager.go 24.00% 18 Missing and 1 partial ⚠️
fracmanager/lifecycle_manager.go 80.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #429      +/-   ##
==========================================
- Coverage   70.83%   70.60%   -0.24%     
==========================================
  Files         225      225              
  Lines       17514    17577      +63     
==========================================
+ Hits        12406    12410       +4     
- Misses       4183     4240      +57     
- Partials      925      927       +2     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

Copy link
Copy Markdown
Contributor

🔴 Performance Degradation

Some benchmarks have degraded compared to the previous run.
Click on Show table button to see full list of degraded benchmarks.

Show table
Name Previous Current Ratio Verdict
AggWide/size=1000000-4 8fb975 2ef516
469.00 B/op 553.00 B/op 1.18 🔴

@eguguchkin eguguchkin modified the milestones: v0.73.0, v0.74.0 Jun 22, 2026
@dkharms dkharms force-pushed the 0-fm-compaction-api branch from d657428 to 58ee0db Compare June 23, 2026 18:47
Comment thread fracmanager/fraction_registry.go Outdated
claimed = append(claimed, s)
}

r.rebuildSnapshot()

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.

No need to rebuild the snapshot here — we're just moving from one queue to another, so the overall set of fractions hasn't changed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yep, agree on this one.

for _, f := range consumed {
info := f.Info()
r.stats.compacting.Sub(info)
delete(r.compacting, info.Name())

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.

Should we additionally check, for reliability, that such a fraction is indeed present in the compacting list?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, consistency check won't hurt -- I've added it.

@dkharms dkharms force-pushed the 0-fm-compaction-api branch from 9c50ff8 to b6e78ca Compare June 24, 2026 10:43
@github-actions

Copy link
Copy Markdown
Contributor

🔴 Performance Degradation

Some benchmarks have degraded compared to the previous run.
Click on Show table button to see full list of degraded benchmarks.

Show table
Name Previous Current Ratio Verdict
MutexListAppend-4 9b461f b7b71a
206.62 MB/s 185.40 MB/s 0.90 🔴
77569546.00 ns/op 86298697.00 ns/op 1.11 🔴

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants