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

Questions about setting the default number of threads on the client #11618

Closed
zhongqishang opened this issue Jan 9, 2024 · 5 comments
Closed
Labels
area/Client Related to Nacos Client SDK contribution welcome kind/enhancement Category issues or prs related to enhancement.

Comments

@zhongqishang
Copy link
Contributor

zhongqishang commented Jan 9, 2024

Is your feature request related to a problem? Please describe.

Nacos is a great component!

We use it with flink/spark/hive in the bigdata hadoop system. Our machine cpu is 80. Each machine has about 60+ containers, which is 60+ JVM instances. According to ThreadUtils.getSuitableThreadCount() The result will be 128. It caused a lot of thread usage.

That is, 128 com.alibaba.nacos.client.Worker and 64 com.alibaba.nacos.client.naming.updater.xx.

For server side, use ThreadUtils.getSuitableThreadCount() to set thread num is suitable.
But for client side, i think this is not a suitable default value.

Now i via -Dnacos.common.processors=2 to set.

Describe the solution you'd like

Add max thread count value config or default thread count value for client side.
BTW, include client side grpc client

Describe alternatives you've considered

None

Additional context

None

@KomachiSion KomachiSion added the kind/discussion Category issues related to discussion label Jan 11, 2024
@KomachiSion
Copy link
Collaborator

I think the suitable thread based on processor number is right logic, but I think client provide config the max count thread size also right.

But the problem is, what's the default max count value? If same with now, default no limit, only when users config the max count, it works. Is it ok?

@zhongqishang
Copy link
Contributor Author

I think the suitable thread based on processor number is right logic, but I think client provide config the max count thread size also right.

But the problem is, what's the default max count value? If same with now, default no limit, only when users config the max count, it works. Is it ok?

I think i get you said the right logic is for the most cloud/container env. right?
I agree with that, but for some phisycal machine deploy multiple app, this default value may not be suitable.

Consider compatible and for most scenarios, and same with now is appropriate. Add a new parameter and it is appropriate to explain it in the documentation.

@KomachiSion
Copy link
Collaborator

If add this configuration to set max thread count, the document must be updated and it's in plan.

The problem is what's the default value of max thread count, if no setting, the value is -1 and means use old logic with ThreadUtils.getSuitableThreadCount() .

@zhongqishang
Copy link
Contributor Author

I think no setting and the default value is -1 and means use old logic is reasonable.
This will be not effect user upgrade client version, and provided a code way to set max thread count.

@KomachiSion
Copy link
Collaborator

I think no setting and the default value is -1 and means use old logic is reasonable. This will be not effect user upgrade client version, and provided a code way to set max thread count.

If no arguemnt, welcome to submit PR to do this.

@KomachiSion KomachiSion added contribution welcome kind/enhancement Category issues or prs related to enhancement. area/Client Related to Nacos Client SDK and removed kind/discussion Category issues related to discussion labels Jan 23, 2024
@KomachiSion KomachiSion modified the milestone: 2.3.1 Jan 23, 2024
zhongqishang added a commit to zhongqishang/nacos that referenced this issue Jan 24, 2024
zhongqishang added a commit to zhongqishang/nacos that referenced this issue Jan 24, 2024
KomachiSion pushed a commit that referenced this issue Jan 31, 2024
… naming polling (#11693)

* [ISSUE #11618] Add the config of max thread count for client worker & naming polling

* Fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/Client Related to Nacos Client SDK contribution welcome kind/enhancement Category issues or prs related to enhancement.
Projects
None yet
Development

No branches or pull requests

2 participants