Skip to content

Use common thread pool only if it is set by user#1

Closed
sazzad16 wants to merge 2 commits intoyangbodong22011:feature-introduce-jedis-thread-factoryfrom
sazzad16:yangbodong22011-feature-introduce-jedis-thread-factory
Closed

Use common thread pool only if it is set by user#1
sazzad16 wants to merge 2 commits intoyangbodong22011:feature-introduce-jedis-thread-factoryfrom
sazzad16:yangbodong22011-feature-introduce-jedis-thread-factory

Conversation

@sazzad16
Copy link

No description provided.

.setKeepAliveMillSecs(DEFAULT_KEEPALIVE_TIME_MS)
.setThreadNamePrefix("jedis-multi-node-pipeline")
.setWorkQueue(new ArrayBlockingQueue<>(DEFAULT_BLOCKING_QUEUE_SIZE)).build();
private final ExecutorService executorService = getThreadPool();
Copy link
Owner

Choose a reason for hiding this comment

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

We need to define executorService as a static variable to avoid multiple creation and destruction.

.setCoreSize(DEFAULT_CORE_POOL_SIZE)
.setMaxSize(DEFAULT_MAXIMUM_POOL_SIZE)
.setKeepAliveMillSecs(DEFAULT_KEEPALIVE_TIME_MS)
.setThreadNamePrefix("jedis-multi-node-pipeline")
Copy link
Owner

Choose a reason for hiding this comment

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

I guess you mean to make it the default thread pool of Jedis, then we need to modify the name to jedis-default-xxx instead of jedis-multi-node-pipeline.

Before, I put it in MultiNodePipeline.java so that it only appears where it is used, and these thread pool parameters are prepared for the jedis-multi-node-pipeline scenario.

.setCoreSize(DEFAULT_CORE_POOL_SIZE)
.setMaxSize(DEFAULT_MAXIMUM_POOL_SIZE)
.setKeepAliveMillSecs(DEFAULT_KEEPALIVE_TIME_MS)
.setThreadNamePrefix("jedis-multi-node-pipeline")
Copy link
Owner

Choose a reason for hiding this comment

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

I guess you mean to make it the default thread pool of Jedis, then we need to modify the name to jedis-default-xxx instead of jedis-multi-node-pipeline.

Before, I put it in MultiNodePipeline.java so that it only appears where it is used, and these thread pool parameters are prepared for the jedis-multi-node-pipeline scenario.

@yangbodong22011 yangbodong22011 force-pushed the feature-introduce-jedis-thread-factory branch from 087ede6 to 44713b7 Compare June 21, 2023 05:54
@sazzad16 sazzad16 closed this Aug 19, 2023
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.

2 participants