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

Add --item-block-worker-count flag to velero install and server #8380

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

sseago
Copy link
Collaborator

@sseago sseago commented Nov 7, 2024

Thank you for contributing to Velero!

Please add a summary of your change

This is the second PR related to phase 2 for backup performance improvements which adds a new flag --item-block-worker-count to velero install and server commands.

Follow-on PRs will use this value to determine the number of goroutines to start for processing ItemBlocks

Does your change fix a particular issue?

Fixes #(issue)

Please indicate you've done the following:

Copy link

codecov bot commented Nov 7, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 58.95%. Comparing base (d0cffa3) to head (6588141).
Report is 26 commits behind head on main.

Files with missing lines Patch % Lines
pkg/cmd/cli/install/install.go 0.00% 7 Missing ⚠️
pkg/install/deployment.go 57.14% 2 Missing and 1 partial ⚠️
pkg/cmd/server/server.go 0.00% 1 Missing ⚠️
pkg/controller/backup_controller.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8380      +/-   ##
==========================================
- Coverage   58.96%   58.95%   -0.01%     
==========================================
  Files         367      367              
  Lines       38895    38919      +24     
==========================================
+ Hits        22933    22945      +12     
- Misses      14500    14511      +11     
- Partials     1462     1463       +1     

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

Copy link
Member

@kaovilai kaovilai left a comment

Choose a reason for hiding this comment

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

From the design it's not clear to me if threads will always be running, or started as needed. Could you clarify?

@sseago
Copy link
Collaborator Author

sseago commented Nov 8, 2024

From the design it's not clear to me if threads will always be running, or started as needed. Could you clarify?

The thread creation won't be part of this PR, but the threads are long-running goroutines with the configured amount always available -- the main reason for this was that when we eventually expand this to allow multiple backups running at once, these goroutines will be shared among the running backups. So if, for example, you've configured 10 concurrent ItemBlocks and 2 concurrent backups, then there will be 10 threads processing blocks from both backups.

(from the design): "The worker pool will be started by calling RunItemBlockWorkers in backupReconciler.SetupWithManager, passing in the worker count and reconciler context"
https://github.com/vmware-tanzu/velero/blob/main/design/backup-performance-improvements.md

@ywk253100 ywk253100 requested a review from Lyndon-Li November 11, 2024 06:16
Copy link
Contributor

@reasonerjt reasonerjt left a comment

Choose a reason for hiding this comment

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

lgtm

@reasonerjt reasonerjt merged commit dacd5ef into vmware-tanzu:main Nov 15, 2024
43 of 45 checks passed
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