From 2c1eab2b8a65663c17bc86df32e209e39757ff24 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 4 Feb 2019 17:44:00 +0000 Subject: [PATCH] Clarify slow cluster-state log messages (#38302) The message `... took [31s] above the warn threshold of 30s` suggests incorrectly that the task took 61 seconds. This commit adds the clarifying words `which is`. --- .../cluster/service/ClusterApplierService.java | 2 +- .../org/elasticsearch/cluster/service/ClusterService.java | 4 ---- .../org/elasticsearch/cluster/service/MasterService.java | 2 +- .../cluster/service/ClusterApplierServiceTests.java | 6 +++--- .../elasticsearch/cluster/service/MasterServiceTests.java | 8 ++++---- 5 files changed, 9 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java b/server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java index 313ff4c6608..e254196caa4 100644 --- a/server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java +++ b/server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java @@ -517,7 +517,7 @@ public class ClusterApplierService extends AbstractLifecycleComponent implements protected void warnAboutSlowTaskIfNeeded(TimeValue executionTime, String source) { if (executionTime.getMillis() > slowTaskLoggingThreshold.getMillis()) { - logger.warn("cluster state applier task [{}] took [{}] above the warn threshold of {}", source, executionTime, + logger.warn("cluster state applier task [{}] took [{}] which is above the warn threshold of {}", source, executionTime, slowTaskLoggingThreshold); } } diff --git a/server/src/main/java/org/elasticsearch/cluster/service/ClusterService.java b/server/src/main/java/org/elasticsearch/cluster/service/ClusterService.java index f66ca073895..f83f2606b14 100644 --- a/server/src/main/java/org/elasticsearch/cluster/service/ClusterService.java +++ b/server/src/main/java/org/elasticsearch/cluster/service/ClusterService.java @@ -19,8 +19,6 @@ package org.elasticsearch.cluster.service; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateApplier; @@ -45,8 +43,6 @@ import java.util.Collections; import java.util.Map; public class ClusterService extends AbstractLifecycleComponent { - private static final Logger logger = LogManager.getLogger(ClusterService.class); - private final MasterService masterService; private final ClusterApplierService clusterApplierService; diff --git a/server/src/main/java/org/elasticsearch/cluster/service/MasterService.java b/server/src/main/java/org/elasticsearch/cluster/service/MasterService.java index beb42fa1c68..c3555928908 100644 --- a/server/src/main/java/org/elasticsearch/cluster/service/MasterService.java +++ b/server/src/main/java/org/elasticsearch/cluster/service/MasterService.java @@ -569,7 +569,7 @@ public class MasterService extends AbstractLifecycleComponent { protected void warnAboutSlowTaskIfNeeded(TimeValue executionTime, String source) { if (executionTime.getMillis() > slowTaskLoggingThreshold.getMillis()) { - logger.warn("cluster state update task [{}] took [{}] above the warn threshold of {}", source, executionTime, + logger.warn("cluster state update task [{}] took [{}] which is above the warn threshold of {}", source, executionTime, slowTaskLoggingThreshold); } } diff --git a/server/src/test/java/org/elasticsearch/cluster/service/ClusterApplierServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/service/ClusterApplierServiceTests.java index 770ae68e128..0d0ed96bf12 100644 --- a/server/src/test/java/org/elasticsearch/cluster/service/ClusterApplierServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/service/ClusterApplierServiceTests.java @@ -195,19 +195,19 @@ public class ClusterApplierServiceTests extends ESTestCase { "test1 shouldn't see because setting is too low", ClusterApplierService.class.getCanonicalName(), Level.WARN, - "*cluster state applier task [test1] took [*] above the warn threshold of *")); + "*cluster state applier task [test1] took [*] which is above the warn threshold of *")); mockAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "test2", ClusterApplierService.class.getCanonicalName(), Level.WARN, - "*cluster state applier task [test2] took [32s] above the warn threshold of *")); + "*cluster state applier task [test2] took [32s] which is above the warn threshold of *")); mockAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "test4", ClusterApplierService.class.getCanonicalName(), Level.WARN, - "*cluster state applier task [test3] took [34s] above the warn threshold of *")); + "*cluster state applier task [test3] took [34s] which is above the warn threshold of *")); Logger clusterLogger = LogManager.getLogger(ClusterApplierService.class); Loggers.addAppender(clusterLogger, mockAppender); diff --git a/server/src/test/java/org/elasticsearch/cluster/service/MasterServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/service/MasterServiceTests.java index 7ed3f45e505..1136ab857ca 100644 --- a/server/src/test/java/org/elasticsearch/cluster/service/MasterServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/service/MasterServiceTests.java @@ -652,25 +652,25 @@ public class MasterServiceTests extends ESTestCase { "test1 shouldn't see because setting is too low", MasterService.class.getCanonicalName(), Level.WARN, - "*cluster state update task [test1] took [*] above the warn threshold of *")); + "*cluster state update task [test1] took [*] which is above the warn threshold of *")); mockAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "test2", MasterService.class.getCanonicalName(), Level.WARN, - "*cluster state update task [test2] took [32s] above the warn threshold of *")); + "*cluster state update task [test2] took [32s] which is above the warn threshold of *")); mockAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "test3", MasterService.class.getCanonicalName(), Level.WARN, - "*cluster state update task [test3] took [33s] above the warn threshold of *")); + "*cluster state update task [test3] took [33s] which is above the warn threshold of *")); mockAppender.addExpectation( new MockLogAppender.SeenEventExpectation( "test4", MasterService.class.getCanonicalName(), Level.WARN, - "*cluster state update task [test4] took [34s] above the warn threshold of *")); + "*cluster state update task [test4] took [34s] which is above the warn threshold of *")); Logger clusterLogger = LogManager.getLogger(MasterService.class); Loggers.addAppender(clusterLogger, mockAppender);