-
Notifications
You must be signed in to change notification settings - Fork 25.9k
Add search task watchdog to log hot threads on slow search #142746
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 11 commits
5e5e2ec
e89be21
fdb151b
eef35d6
61e67b8
c4f3093
874b19e
6f9f7a7
0eb8018
b0a4c2b
ffde4fe
57f95d4
94079f9
d1a0d29
9d9c642
a625979
017fff8
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 |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| area: Search | ||
| issues: [] | ||
| pr: 142746 | ||
| summary: Add search task watchdog to log hot threads on slow search | ||
| type: enhancement |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,211 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the "Elastic License | ||
| * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side | ||
| * Public License v 1"; you may not use this file except in compliance with, at | ||
| * your election, the "Elastic License 2.0", the "GNU Affero General Public | ||
| * License v3.0 only", or the "Server Side Public License, v 1". | ||
| */ | ||
|
|
||
| package org.elasticsearch.action.search; | ||
|
|
||
| import org.apache.logging.log4j.Level; | ||
| import org.apache.logging.log4j.LogManager; | ||
| import org.apache.logging.log4j.Logger; | ||
| import org.elasticsearch.common.ReferenceDocs; | ||
| import org.elasticsearch.common.component.AbstractLifecycleComponent; | ||
| import org.elasticsearch.common.settings.ClusterSettings; | ||
| import org.elasticsearch.common.settings.Setting; | ||
| import org.elasticsearch.common.settings.Settings; | ||
| import org.elasticsearch.core.TimeValue; | ||
| import org.elasticsearch.monitor.jvm.HotThreads; | ||
| import org.elasticsearch.tasks.TaskManager; | ||
| import org.elasticsearch.threadpool.ThreadPool; | ||
|
|
||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
|
|
||
| import static org.elasticsearch.core.Strings.format; | ||
|
|
||
| /** | ||
| * Monitors search tasks for slow execution and logs hot threads when thresholds are exceeded. | ||
| * <p> | ||
| * On data nodes, logs hot threads when a shard-level search operation (query/fetch phase) exceeds | ||
| * the data node threshold. On coordinator nodes, logs hot threads when the reduce/merge phase | ||
| * exceeds the coordinator threshold (only after all shards have responded). | ||
| * <p> | ||
| * This helps diagnose slow searches by capturing what threads are doing while the search is | ||
| * still running, rather than just logging after completion. | ||
| */ | ||
| public class SearchTaskWatchdog extends AbstractLifecycleComponent { | ||
|
|
||
| private static final Logger logger = LogManager.getLogger(SearchTaskWatchdog.class); | ||
|
|
||
| public static final Setting<Boolean> ENABLED = Setting.boolSetting( | ||
| "search.task_watchdog.enabled", | ||
| false, | ||
| Setting.Property.NodeScope, | ||
| Setting.Property.Dynamic | ||
| ); | ||
|
|
||
| public static final Setting<TimeValue> COORDINATOR_THRESHOLD = Setting.timeSetting( | ||
| "search.task_watchdog.coordinator_threshold", | ||
| TimeValue.timeValueSeconds(3), | ||
| TimeValue.MINUS_ONE, | ||
| Setting.Property.NodeScope, | ||
| Setting.Property.Dynamic | ||
| ); | ||
|
|
||
| public static final Setting<TimeValue> DATA_NODE_THRESHOLD = Setting.timeSetting( | ||
| "search.task_watchdog.data_node_threshold", | ||
| TimeValue.timeValueSeconds(3), | ||
| TimeValue.MINUS_ONE, | ||
| Setting.Property.NodeScope, | ||
| Setting.Property.Dynamic | ||
| ); | ||
|
|
||
| public static final Setting<TimeValue> INTERVAL = Setting.timeSetting( | ||
| "search.task_watchdog.interval", | ||
| TimeValue.timeValueSeconds(1), | ||
| TimeValue.timeValueMillis(100), | ||
| Setting.Property.NodeScope, | ||
| Setting.Property.Dynamic | ||
| ); | ||
|
|
||
| public static final Setting<TimeValue> COOLDOWN_PERIOD = Setting.timeSetting( | ||
| "search.task_watchdog.cooldown_period", | ||
| TimeValue.timeValueSeconds(30), | ||
| TimeValue.ZERO, | ||
|
spinscale marked this conversation as resolved.
|
||
| Setting.Property.NodeScope, | ||
| Setting.Property.Dynamic | ||
| ); | ||
|
|
||
| private final TaskManager taskManager; | ||
| private final ThreadPool threadPool; | ||
|
|
||
| private volatile boolean enabled; | ||
| private volatile long coordinatorThresholdNanos; | ||
| private volatile long dataNodeThresholdNanos; | ||
| private volatile long minThresholdNanos; | ||
| private volatile TimeValue interval; | ||
| private volatile long cooldownPeriodNanos; | ||
| private volatile long lastLoggedNanos = 0; | ||
| private final AtomicBoolean scheduled = new AtomicBoolean(false); | ||
|
|
||
| public SearchTaskWatchdog(Settings settings, ClusterSettings clusterSettings, TaskManager taskManager, ThreadPool threadPool) { | ||
| this.taskManager = taskManager; | ||
| this.threadPool = threadPool; | ||
|
|
||
| this.enabled = ENABLED.get(settings); | ||
|
Contributor
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. aren't all of these called with the settings update consumer anyway, so no need to call twice?
Contributor
Author
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. I'm not sure I understand. What are we calling twice?
Contributor
Author
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. Perhaps using intializeAndWatch is what you meant here? d1a0d29
Contributor
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. yes! |
||
| this.coordinatorThresholdNanos = COORDINATOR_THRESHOLD.get(settings).nanos(); | ||
| this.dataNodeThresholdNanos = DATA_NODE_THRESHOLD.get(settings).nanos(); | ||
| this.minThresholdNanos = computeMinThreshold(coordinatorThresholdNanos, dataNodeThresholdNanos); | ||
| this.interval = INTERVAL.get(settings); | ||
| this.cooldownPeriodNanos = COOLDOWN_PERIOD.get(settings).nanos(); | ||
|
|
||
| clusterSettings.addSettingsUpdateConsumer(ENABLED, this::setEnabled); | ||
| clusterSettings.addSettingsUpdateConsumer(COORDINATOR_THRESHOLD, v -> setCoordinatorThreshold(v.nanos())); | ||
| clusterSettings.addSettingsUpdateConsumer(DATA_NODE_THRESHOLD, v -> setDataNodeThreshold(v.nanos())); | ||
| clusterSettings.addSettingsUpdateConsumer(INTERVAL, v -> this.interval = v); | ||
| clusterSettings.addSettingsUpdateConsumer(COOLDOWN_PERIOD, v -> this.cooldownPeriodNanos = v.nanos()); | ||
| } | ||
|
|
||
| private void setEnabled(boolean enabled) { | ||
| this.enabled = enabled; | ||
| if (enabled && lifecycle.started()) { | ||
| scheduleNext(); | ||
| } | ||
| } | ||
|
|
||
| private void setCoordinatorThreshold(long newCoordinatorThresholdValue) { | ||
| this.coordinatorThresholdNanos = newCoordinatorThresholdValue; | ||
| this.minThresholdNanos = computeMinThreshold(newCoordinatorThresholdValue, dataNodeThresholdNanos); | ||
|
Contributor
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. no need to pass variables, as you set
Contributor
Author
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. I think it's more readable with parameters (i.e. it conveys what it does without having to step inside the method)
Contributor
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. haha, for me it was the opposite. As we're essentially testing I don't have a strong preference though. |
||
| } | ||
|
|
||
| private void setDataNodeThreshold(long newDataNodeThresholdValue) { | ||
| this.dataNodeThresholdNanos = newDataNodeThresholdValue; | ||
| this.minThresholdNanos = computeMinThreshold(coordinatorThresholdNanos, newDataNodeThresholdValue); | ||
| } | ||
|
|
||
| private static long computeMinThreshold(long coordinatorNanos, long dataNodeNanos) { | ||
| long coordValue = coordinatorNanos > 0 ? coordinatorNanos : Long.MAX_VALUE; | ||
| long dataValue = dataNodeNanos > 0 ? dataNodeNanos : Long.MAX_VALUE; | ||
| return Math.min(coordValue, dataValue); | ||
| } | ||
|
|
||
| @Override | ||
| protected void doStart() { | ||
| if (enabled) { | ||
| scheduleNext(); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| protected void doStop() {} | ||
|
|
||
| @Override | ||
| protected void doClose() {} | ||
|
|
||
| private void scheduleNext() { | ||
| if (enabled && lifecycle.stoppedOrClosed() == false && scheduled.compareAndSet(false, true)) { | ||
| threadPool.scheduleUnlessShuttingDown(interval, threadPool.generic(), this::run); | ||
| } | ||
| } | ||
|
|
||
| private void run() { | ||
| try { | ||
| if (enabled == false || lifecycle.stoppedOrClosed()) { | ||
| return; | ||
| } | ||
|
|
||
| final long now = threadPool.relativeTimeInNanos(); | ||
|
|
||
| // skip task iteration if in cooldown period | ||
|
andreidan marked this conversation as resolved.
Outdated
|
||
| if (lastLoggedNanos > 0 && (now - lastLoggedNanos) < cooldownPeriodNanos) { | ||
| return; | ||
| } | ||
|
|
||
| if (minThresholdNanos < Long.MAX_VALUE) { | ||
| taskManager.forEachCancellableTask(minThresholdNanos, info -> { | ||
| try { | ||
| handleTask(info, now); | ||
| } catch (Exception e) { | ||
| logger.debug(() -> "error processing task [" + info.task().getId() + "]", e); | ||
| } | ||
| // we logged a slow task in this iteration, so skip checking other tasks | ||
| // as we're now in the cooldown period | ||
| return lastLoggedNanos != now; | ||
| }); | ||
| } | ||
| } finally { | ||
| scheduled.set(false); | ||
| scheduleNext(); | ||
| } | ||
| } | ||
|
|
||
| private void handleTask(TaskManager.CancellableTaskInfo info, long now) { | ||
| assert ThreadPool.assertCurrentThreadPool(ThreadPool.Names.GENERIC); | ||
| if (info.task() instanceof SearchShardTask) { | ||
| if (dataNodeThresholdNanos > 0 && info.elapsedNanos() > dataNodeThresholdNanos) { | ||
|
spinscale marked this conversation as resolved.
|
||
| logSlowTask(info, "shard", now); | ||
| } | ||
| } else if (info.task() instanceof SearchTask) { | ||
| if (coordinatorThresholdNanos > 0 | ||
| && info.elapsedNanos() > coordinatorThresholdNanos | ||
| && info.hasOutstandingChildren() == false) { | ||
| logSlowTask(info, "coordinator", now); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private void logSlowTask(TaskManager.CancellableTaskInfo info, String type, long now) { | ||
| lastLoggedNanos = now; | ||
|
|
||
| long taskId = info.task().getId(); | ||
| HotThreads.logLocalHotThreads( | ||
| logger, | ||
| Level.INFO, | ||
| format("slow search %s task [%d] parent [%s]", type, taskId, info.task().getParentTaskId()), | ||
| ReferenceDocs.SEARCH_TASK_WATCHDOG | ||
| ); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.