Skip to content

[Distributed] Fix init serialToParallelPool bug if cpu.cores > 100#2362

Merged
jt2594838 merged 2 commits intoapache:masterfrom
OneSizeFitsQuorum:fix_serialToParallelPool_init_bug
Dec 29, 2020
Merged

[Distributed] Fix init serialToParallelPool bug if cpu.cores > 100#2362
jt2594838 merged 2 commits intoapache:masterfrom
OneSizeFitsQuorum:fix_serialToParallelPool_init_bug

Conversation

@OneSizeFitsQuorum
Copy link
Contributor

@OneSizeFitsQuorum OneSizeFitsQuorum commented Dec 28, 2020

We will encounter a exception when starting cluster server if cpu.cores > 100, so we should change the parameters for ThreadPoolExecutor. As serialToParallelPool will only do heartbeat or add/remove node, I think the coreSize can be 'allNode.size()', and the maxSize can be set to Math.max(allNodes.size(), Runtime.getRuntime().availableProcessors()).

Copy link
Member

@neuyilan neuyilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good Job! besides, envy that you have more than 100 cores servers~
But what about the server's CPUs less than the allNodes.size()?

I think we can reference the code when creating RaftServer[1]

int maxConcurrentClientNum = Math.max(CommonUtils.getCpuCores(), config.getMaxConcurrentClientNum()); TThreadPoolServer.Args poolArgs = new TThreadPoolServer.Args(socket).maxWorkerThreads(maxConcurrentClientNum) .minWorkerThreads(CommonUtils.getCpuCores());
[1]
https://github.com/apache/iotdb/blob/master/cluster/src/main/java/org/apache/iotdb/cluster/utils/ClusterUtils.java#L242

@OneSizeFitsQuorum
Copy link
Contributor Author

OneSizeFitsQuorum commented Dec 28, 2020

Good Job! besides, envy that you have more than 100 cores servers~

Scale up is too nice, I've forgotten how to scale out~

@OneSizeFitsQuorum
Copy link
Contributor Author

OneSizeFitsQuorum commented Dec 28, 2020

int maxConcurrentClientNum = Math.max(CommonUtils.getCpuCores(), config.getMaxConcurrentClientNum());

You are right.But I think the core size can be smaller than cpu.core. What about this? serialToParallelPool = new ThreadPoolExecutor(allNodes.size(), Math.max(allNodes.size(), Runtime.getRuntime().availableProcessors()), 1000L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>(), new ThreadFactoryBuilder().setNameFormat(getName() + "-SerialToParallel%d").build());

@neuyilan neuyilan closed this Dec 29, 2020
@neuyilan
Copy link
Member

int maxConcurrentClientNum = Math.max(CommonUtils.getCpuCores(), config.getMaxConcurrentClientNum());

You are right.But I think the core size can be smaller than cpu.core. What about this? serialToParallelPool = new ThreadPoolExecutor(allNodes.size(), Math.max(allNodes.size(), Runtime.getRuntime().availableProcessors()), 1000L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>(), new ThreadFactoryBuilder().setNameFormat(getName() + "-SerialToParallel%d").build());

LGTM

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jt2594838 jt2594838 merged commit 10ae899 into apache:master Dec 29, 2020
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.

3 participants