-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Add a dedicated threadpool for node connections #30150
Changes from all commits
26acc57
c7f14cf
b45c981
20fe504
1798e9d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,6 @@ | |
|
||
import org.apache.logging.log4j.message.ParameterizedMessage; | ||
import org.apache.lucene.util.Counter; | ||
import org.elasticsearch.core.internal.io.IOUtils; | ||
import org.elasticsearch.Version; | ||
import org.elasticsearch.common.Nullable; | ||
import org.elasticsearch.common.component.AbstractComponent; | ||
|
@@ -38,6 +37,7 @@ | |
import org.elasticsearch.common.util.concurrent.XRejectedExecutionHandler; | ||
import org.elasticsearch.common.xcontent.ToXContentFragment; | ||
import org.elasticsearch.common.xcontent.XContentBuilder; | ||
import org.elasticsearch.core.internal.io.IOUtils; | ||
import org.elasticsearch.node.Node; | ||
|
||
import java.io.Closeable; | ||
|
@@ -79,6 +79,7 @@ public static class Names { | |
public static final String FORCE_MERGE = "force_merge"; | ||
public static final String FETCH_SHARD_STARTED = "fetch_shard_started"; | ||
public static final String FETCH_SHARD_STORE = "fetch_shard_store"; | ||
public static final String NODE_CONNECTIONS = "node_connections"; | ||
} | ||
|
||
public enum ThreadPoolType { | ||
|
@@ -135,6 +136,7 @@ public static ThreadPoolType fromType(String type) { | |
map.put(Names.FORCE_MERGE, ThreadPoolType.FIXED); | ||
map.put(Names.FETCH_SHARD_STARTED, ThreadPoolType.SCALING); | ||
map.put(Names.FETCH_SHARD_STORE, ThreadPoolType.SCALING); | ||
map.put(Names.NODE_CONNECTIONS, ThreadPoolType.SCALING); | ||
THREAD_POOL_TYPES = Collections.unmodifiableMap(map); | ||
} | ||
|
||
|
@@ -186,6 +188,7 @@ public ThreadPool(final Settings settings, final ExecutorBuilder<?>... customBui | |
builders.put(Names.FETCH_SHARD_STARTED, new ScalingExecutorBuilder(Names.FETCH_SHARD_STARTED, 1, 2 * availableProcessors, TimeValue.timeValueMinutes(5))); | ||
builders.put(Names.FORCE_MERGE, new FixedExecutorBuilder(settings, Names.FORCE_MERGE, 1, -1)); | ||
builders.put(Names.FETCH_SHARD_STORE, new ScalingExecutorBuilder(Names.FETCH_SHARD_STORE, 1, 2 * availableProcessors, TimeValue.timeValueMinutes(5))); | ||
builders.put(Names.NODE_CONNECTIONS, new ScalingExecutorBuilder(Names.NODE_CONNECTIONS, 1, 2 * availableProcessors, TimeValue.timeValueMinutes(5))); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This feels too small to me, for larger clusters. Connecting to a node is not a resource-intensive operation but it might take a long time to time out, blocking other connection attempts. Perhaps this is a reasonable default and we can note in the docs that larger clusters may prefer to increase this limit? |
||
for (final ExecutorBuilder<?> builder : customBuilders) { | ||
if (builders.containsKey(builder.name())) { | ||
throw new IllegalArgumentException("builder with name [" + builder.name() + "] already exists"); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the
ConnectionChecker
below could also reasonably run on theNODE_CONNECTIONS
threadpool.