-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add allocation write load stats to write thread pool #130373
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
Changes from 2 commits
4037ccf
b8d8e56
fd8f602
47debcd
37b20e8
be50439
45f6836
bd2e0f1
629ca35
b0fdaf2
46a659b
3ecac48
4986fa5
7ded14f
f942332
f9488fe
5057324
fb05d84
326f9e1
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 |
|---|---|---|
|
|
@@ -217,6 +217,16 @@ public static ThreadPoolType fromType(String type) { | |
| // moving average is 100ms, and we get one task which takes 20s the new EWMA will be ~500ms. | ||
| public static final double DEFAULT_INDEX_AUTOSCALING_EWMA_ALPHA = 0.02; | ||
|
|
||
| /** | ||
| * If the queue latency reaches a high value (e.g. 10-30 seconds), then this thread pool is overwhelmed. It may be temporary, but that | ||
| * spike warrants the allocation balancer adjusting some number of shards, if possible. Therefore, it is alright to react quickly. | ||
| * | ||
| * As an example, suppose the EWMA is 10_000ms, i.e. 10 seconds. | ||
| * A single task in the queue that takes 30_000ms, i.e. 30 seconds, would result in a new EWMA of ~12_000ms | ||
| * 0.1 x 30_000ms + 0.9 x 10_000 = 3_000ms + 9_000ms = 12_000ms | ||
| */ | ||
| public static final double DEFAULT_WRITE_THREAD_POOL_QUEUE_LATENCY_EWMA_ALPHA = 0.1; | ||
|
||
|
|
||
| private final Map<String, ExecutorHolder> executors; | ||
|
|
||
| private final ThreadPoolInfo threadPoolInfo; | ||
|
|
@@ -271,6 +281,17 @@ public Collection<ExecutorBuilder> builders() { | |
| Setting.Property.NodeScope | ||
| ); | ||
|
|
||
| /** | ||
| * The {@link org.elasticsearch.common.ExponentiallyWeightedMovingAverage} alpha for tracking task queue latency. | ||
| */ | ||
| public static final Setting<Double> WRITE_THREAD_POOL_QUEUE_LATENCY_EWMA_ALPHA = Setting.doubleSetting( | ||
| "thread_pool.task_tracking.queue_latency.ewma_alpha", | ||
| DEFAULT_WRITE_THREAD_POOL_QUEUE_LATENCY_EWMA_ALPHA, | ||
| 0, | ||
| 1, | ||
| Setting.Property.NodeScope | ||
| ); | ||
|
|
||
| /** | ||
| * Defines and builds the many thread pools delineated in {@link Names}. | ||
| * | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Nit: I think when you get to three consecutive booleans, it's worth making a builder for readability, e.g.
A side benefit would be you could default all to false and just specify the ones that were true.
Happy to be challenged on that.
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.
If we decide this is a good idea, also happy for it to be a separate PR because it'll add a fair bit of noise I imagine
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.
Sure, I'll put up a separate PR for this
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.
Alternatively I wonder if the max latency tracking is so cheap that we don't need to have a config for it. The ongoing task tracking is quite expensive, so it makes sense to be able to opt out of it. It would also reduce the surface area of this change.
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.
It seems a waste to track information unnecessarily, and potentially confusing wondering why it's not used. I'd prefer not to. Also never know how code will grow over time.
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.
To close this out, I've merged a TaskTrackingConfig Builder subclass and updated the callers with it.