From 0ebc363d93ec01fc06d916ec83b899109e1e5d01 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 4 Feb 2019 08:58:58 +0000 Subject: [PATCH 1/2] Clarify slow cluster-state log messages 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`. --- .../elasticsearch/cluster/service/ClusterApplierService.java | 2 +- .../org/elasticsearch/cluster/service/ClusterService.java | 4 ---- .../java/org/elasticsearch/cluster/service/MasterService.java | 2 +- 3 files changed, 2 insertions(+), 6 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 313ff4c660866..e254196caa47b 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 void onSuccess(String source) { 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 f66ca0738954b..f83f2606b14b6 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.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 beb42fa1c6814..c355592890855 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 TimeValue ackTimeout() { 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); } } From 8cc4b0dd9cff4bd07c135b5ca15f8e1ce971f02d Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 4 Feb 2019 14:56:18 +0000 Subject: [PATCH 2/2] Fix tests --- .../cluster/service/ClusterApplierServiceTests.java | 6 +++--- .../elasticsearch/cluster/service/MasterServiceTests.java | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) 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 770ae68e1285f..0d0ed96bf12aa 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 void testLongClusterStateUpdateLogging() throws Exception { "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 7ed3f45e505f9..1136ab857ca4e 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 void testLongClusterStateUpdateLogging() throws Exception { "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);