8000 Sidecar should support configure the number of worker threads to run · Issue #11268 · istio/istio · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Sidecar should support configure the number of worker threads to run #11268

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

Closed
hzxuzhonghu opened this issue Jan 25, 2019 · 6 comments
Closed

Comments

@hzxuzhonghu
Copy link
Member
hzxuzhonghu commented Jan 25, 2019

We guys from Huawei have tested how the --concurrency flag influence Envoy performance.

And the result shows that when we configure concurrency the number of cpu cores, it supports the maximum qps and takes up less cpu.

--concurrency The number of worker threads to run. If not specified defaults to the number of hardware threads on the machine.

But as we know Envoy in a container can see all cores in host machine moby/moby#20770

So one thought come to mind is we can infer the number of cores from container cpu request, and pass this value to sidecar, so that pilot-agent can start envoy with this legitimate value.

@hzxuzhonghu hzxuzhonghu changed the title SidecarSupport configure the number of worker threads to run Sidecar should support configure the number of worker threads to run Jan 25, 2019
@hzxuzhonghu
Copy link
Member Author

@ymesika
Copy link
Member
ymesika commented Jan 27, 2019

According to the internal doc by default the concurrency value is 0 and in this case it will use the number of cores on the machine.
How is it different from what you suggest?

@hzxuzhonghu
Copy link
Member Author

But as we know Envoy in a container can see all cores in host machine moby/moby#20770

This value is not accurate in a docker container.

@wenchenglu
Copy link
Contributor

@mandarjog should this be marked for 1.1. Sounds a reasonable change to go into 1.1 for performance improvement.

@duderino fyi

@wenchenglu wenchenglu added this to the 1.1 milestone Jan 30, 2019
@hzxuzhonghu
Copy link
Member Author

Thanks @wenchenglu

@knrc
Copy link
knrc commented Feb 1, 2019

FYI this was already under discussion through #8938, already in the 1.1 bucket

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants
0