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

Slow backup because of the little QPS and burst numbers #7806

Closed
dariodsa opened this issue May 18, 2024 · 5 comments · Fixed by #8166
Closed

Slow backup because of the little QPS and burst numbers #7806

dariodsa opened this issue May 18, 2024 · 5 comments · Fixed by #8166
Assignees
Labels
Milestone

Comments

@dariodsa
Copy link

What steps did you take and what happened:
A Velero process is setting up the Factory settings if a user is passing client burst and client qps parameters, so that later on if a KubeClient is requested from the Factory instance, KubeClient will have client burst and client qps parameters as intended. So if a user pass a client-burst or client-qps parameters those parameters will be added in the Factory settings of a main Velero process.

f.SetClientQPS(config.clientQPS)
if config.clientBurst <= 0 {
return nil, errors.New("client-burst must be positive")
}
f.SetClientBurst(config.clientBurst)

Also Velero have 5 minutes cache of a discovered resources, but all of that is not used in the plugin section of a code.
In pkg/cmd/server/plugin/plugin.go there is a multiple creation of instance discoveryHelper so when velero-plugin process is running it does discovery multiple times (newServiceAccountBackupItemAction function, newRemapCRDVersionAction function).

clientset, err := f.KubeClient()
if err != nil {
return nil, err
}
discoveryHelper, err := velerodiscovery.NewHelper(clientset.Discovery(), logger)

clientset, err := f.KubeClient()
if err != nil {
return nil, err
}
discoveryHelper, err := velerodiscovery.NewHelper(clientset.Discovery(), logger)
if err != nil {
return nil, err
}

Also velero-plugin is running in a seperate process so factory settings like clientQPS or clientBurst are not passed. So KubeClient that is being created in the Plugin section are using default values of client-go library which are around 10 qps. By simply running only discovery in the plugin section only once and adding default values in the Factory instance, Velero backups can actually end up in only a couple of seconds, instead of 10-40 seconds.
To prove that we try adding default values in the Factory instance as you can see down there and our backups were around 10* times faster. We suggest to add default values of a clientBurst and clientQPS parameters direct into the Factory creation function.(newFactory) or passing the client-burst and client-qps parameters to the velero-plugin process by using flags, the same way as the Velero main process is using.
Suggestion

pkg/client/factory.go
func NewFactory(baseName string, config VeleroConfig) Factory {
	f := &factory{
		flags:    pflag.NewFlagSet("", pflag.ContinueOnError),
		baseName: baseName,
	}
        f.clientQPS = 100;  // those numbers are already default in the main Velero process
        f.clientBurst = 100;  // those numbers are already default in the main Velero process
....

I did a presentation about that on DORS/CLUC conference in Zagreb. You can check details here.

  • 👍 for "I would like to see this bug fixed as soon as possible"
  • 👎 for "There are more important bugs to focus on right now"
@Lyndon-Li Lyndon-Li added Performance Area/Plugins Issues related to plugin infra/internal plugins labels May 20, 2024
@Lyndon-Li Lyndon-Li self-assigned this May 20, 2024
@sseago
Copy link
Collaborator

sseago commented May 20, 2024

Nice. We'd probably want to use the same settings from the main velero process if possible -- so if a user changes those from 100 to some higher or lower value via the server param, we should use the same here.

@shubham-pampattiwar
Copy link
Collaborator

@sseago or do we want to decouple the qps and burst settings for client used in plugins i.e. add separate configurable params for plugin client configuration ?

@sseago
Copy link
Collaborator

sseago commented May 20, 2024

@shubham-pampattiwar as long as it's configurable, I'm not sure I have a strong opinion on whether we use the same settings for both or different ones -- but bumping it up from 10 to 100 without providing an option to change it could result in problems for clusters with slow APIServers.

@kaovilai
Copy link
Member

Plugins on startup could perhaps read the velero deployment container arguments once so it can try use the same settings.

@reasonerjt
Copy link
Contributor

I think it makes more sense if we can figure out how to make settings in velero pod visible to plugins.

@reasonerjt reasonerjt assigned ywk253100 and unassigned Lyndon-Li Jun 21, 2024
@reasonerjt reasonerjt added this to the v1.15 milestone Jul 24, 2024
@weshayutin weshayutin added this to OADP Aug 20, 2024
ywk253100 added a commit to ywk253100/velero that referenced this issue Aug 30, 2024
Pass Velero server command args to the plugins

Fixes vmware-tanzu#7806

Signed-off-by: Wenkai Yin(尹文开) <[email protected]>
ywk253100 added a commit to ywk253100/velero that referenced this issue Aug 30, 2024
Pass Velero server command args to the plugins

Fixes vmware-tanzu#7806

Signed-off-by: Wenkai Yin(尹文开) <[email protected]>
ywk253100 added a commit to ywk253100/velero that referenced this issue Aug 30, 2024
Pass Velero server command args to the plugins

Fixes vmware-tanzu#7806

Signed-off-by: Wenkai Yin(尹文开) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants