Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(distribution): Backport distribution fix 'add bounded concurrency for tag lookup and untag #21652

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thcdrt
Copy link
Contributor

@thcdrt thcdrt commented Feb 19, 2025

Comprehensive Summary of your change

Hello,

Investigating about a garbage collection taking months/years to finish, I checked the GC logs and found out that the manifest deletion always took ~30s.
So I asked if some people had the same issue on slack and I found the issue #12948 talking about it and linking to a docker distribution PR fixing the issue.
Problem is that it will be probably only be released on Distribution v3 while this version is still in working state after years of work, that's why I decided to cherry-pick the fix distribution/distribution#4329 written by @microyahoo to improve garbage collection manifest deletion.

I tested the fix, and I went from 30s to ~7s to delete a manifest.

A new configuration has been added on distribution configuration side to configure tag deletion concurrency limit, by default it's set to runtime.GOMAXPROCS(0), so it's working without having to modify docker distribution configuration (no need to modify the Helm Chart).

Issue being fixed

Fixes #12948

Please indicate you've done the following:

  • Well Written Title and Summary of the PR
  • Label the PR as needed. "release-note/ignore-for-release, release-note/new-feature, release-note/update, release-note/enhancement, release-note/community, release-note/breaking-change, release-note/docs, release-note/infra, release-note/deprecation"
  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Made sure tests are passing and test coverage is added if needed.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

@thcdrt thcdrt requested a review from a team as a code owner February 19, 2025 13:53
Copy link

codecov bot commented Feb 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.22%. Comparing base (c8c11b4) to head (664a012).
Report is 390 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #21652      +/-   ##
==========================================
+ Coverage   45.36%   46.22%   +0.86%     
==========================================
  Files         244      247       +3     
  Lines       13333    13883     +550     
  Branches     2719     2875     +156     
==========================================
+ Hits         6049     6418     +369     
- Misses       6983     7128     +145     
- Partials      301      337      +36     
Flag Coverage Δ
unittests 46.22% <ø> (+0.86%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 491 files with indirect coverage changes

@thcdrt thcdrt added the release-note/enhancement Label to mark PR to be added under release notes as enhancement label Feb 19, 2025
@thcdrt thcdrt force-pushed the dev/tacouder/12498/improve-v2-manifest-deletion-performance branch from 33d0189 to 81e99d4 Compare February 19, 2025 14:02
@thcdrt thcdrt force-pushed the dev/tacouder/12498/improve-v2-manifest-deletion-performance branch from 81e99d4 to 664a012 Compare February 19, 2025 14:03
@Vad1mo
Copy link
Member

Vad1mo commented Feb 20, 2025

@thcdrt, did you try using Distribution v3?

@thcdrt
Copy link
Contributor Author

thcdrt commented Feb 20, 2025

No I didn't want to take the risk as it's not released yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/enhancement Label to mark PR to be added under release notes as enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GC performance] The performance of v2 manifest deletion is not good in S3 environment
4 participants