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

limit the number of concurrent executions of the ovs-vsctl command #3263

Closed
wants to merge 7 commits into from

Conversation

qiutingjun
Copy link
Contributor

@qiutingjun qiutingjun commented Sep 27, 2023

What type of this PR

Examples of user facing changes:

  • Features

add rate limiter to limit the number of concurrent executions of the ovs-vsctl command

Which issue(s) this PR fixes:

Fixes #3210

WHAT

🤖 Generated by Copilot at 85a248c

This pull request adds a new feature to rate limit ovs-vsctl commands in the cniserver module, using the ovs package and a new flag --ovs-vsctl-rate. This improves the performance and stability of the OVS integration with kube-ovn. The pull request also updates the helm chart values and the deployment yamls to reflect the new flag and its default value.

🤖 Generated by Copilot at 85a248c

ovs-vsctl rate
limiting improves OVS
stability in spring

HOW

🤖 Generated by Copilot at 85a248c

  • Add a new flag --ovs-vsctl-rate to the ovncni daemonset container, which sets the rate limit of ovs-vsctl commands (link, link, link)
  • Add a new value OVS_VSCTL_RATE to the helm chart values, which sets the default rate limit of ovs-vsctl commands to 100 per second (link)
  • Add a new field OVSVsctlRate to the Configuration struct, which stores the rate limit of ovs-vsctl commands from the flag (link)
  • Define and parse the --ovs-vsctl-rate flag in the ParseFlags function of the config module, and assign its value to the OVSVsctlRate field (link, link)
  • Import the ovs package to the cniserver module, and call the UpdateOVSVsctlLimiter function from the ovs package, which updates the rate limiter with the value from the OVSVsctlRate field (link, link)
  • Import the context and rate packages to the ovs package, and define and initialize a global rate limiter with a default rate and burst of 100 per second (link, link, link)
  • Modify the Exec function of the ovs package, which executes ovs-vsctl commands, to use a context with a timeout of one second, and wait for the rate limiter to allow the execution (link)

@qiutingjun qiutingjun force-pushed the rate branch 2 times, most recently from 55a118a to 424f1da Compare September 27, 2023 07:19
@oilbeater
Copy link
Collaborator

The rate limiter cannot limit the max concurrent processes as we expected. What we need is allow a maximum of 10 ovs-vsctl processes to run simultaneously, with the 11th process being queued until one of the current processes has ended. The rate limiter will give token in a given time regardless of the current processes, the total running processes may still exceed the max number allowed.

@qiutingjun qiutingjun force-pushed the rate branch 4 times, most recently from 17092ef to a45713a Compare October 8, 2023 01:34
pkg/ovs/ovs-vsctl.go Outdated Show resolved Hide resolved
@qiutingjun qiutingjun marked this pull request as draft October 8, 2023 02:47
@qiutingjun qiutingjun marked this pull request as ready for review October 8, 2023 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kube-ovn-cni needs to limit the number of concurrent executions of the ovs-vsctl command
3 participants