vmware-tanzu / velero

Backup and migrate Kubernetes applications and their persistent volumes
https://velero.io
Apache License 2.0
8.78k stars 1.41k forks source link

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

Closed sseago closed 1 week ago

sseago commented 2 weeks ago

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:

codecov[bot] commented 2 weeks ago

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 :warning:
pkg/install/deployment.go 57.14% 2 Missing and 1 partial :warning:
pkg/cmd/server/server.go 0.00% 1 Missing :warning:
pkg/controller/backup_controller.go 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sseago commented 2 weeks ago

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