From 376d1662ac005edae56562ba74b935ce71931b0f Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Thu, 11 Jul 2024 16:37:31 +0200 Subject: [PATCH 1/2] Skip stacktrace for exceptions when not relevant. These exceptions are expected and don't imply bugs, their stacktrace isn't relevant and should not be logged. Relates to ES-7097 --- .../action/NoShardAvailableActionException.java | 8 +++----- .../elasticsearch/action/UnavailableShardsException.java | 5 +++++ .../cluster/block/ClusterBlockException.java | 5 +++++ .../common/breaker/CircuitBreakingException.java | 5 +++++ .../util/concurrent/EsRejectedExecutionException.java | 5 +++++ .../discovery/MasterNotDiscoveredException.java | 5 +++++ .../java/org/elasticsearch/node/NodeClosedException.java | 5 +++++ .../search/aggregations/MultiBucketConsumerService.java | 5 +++++ .../transport/NodeNotConnectedException.java | 5 +++++ 9 files changed, 43 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/NoShardAvailableActionException.java b/server/src/main/java/org/elasticsearch/action/NoShardAvailableActionException.java index e018cf48fcefc..564055aa36750 100644 --- a/server/src/main/java/org/elasticsearch/action/NoShardAvailableActionException.java +++ b/server/src/main/java/org/elasticsearch/action/NoShardAvailableActionException.java @@ -18,8 +18,6 @@ public final class NoShardAvailableActionException extends ElasticsearchException { - private static final StackTraceElement[] EMPTY_STACK_TRACE = new StackTraceElement[0]; - // This is set so that no StackTrace is serialized in the scenario when we wrap other shard failures. // It isn't necessary to serialize this field over the wire as the empty stack trace is serialized instead. private final boolean onShardFailureWrapper; @@ -57,8 +55,8 @@ public NoShardAvailableActionException(StreamInput in) throws IOException { } @Override - public StackTraceElement[] getStackTrace() { - return onShardFailureWrapper ? EMPTY_STACK_TRACE : super.getStackTrace(); + public Throwable fillInStackTrace() { + return this; // this exception doesn't imply a bug, no need for a stack trace } @Override @@ -67,7 +65,7 @@ public void printStackTrace(PrintWriter s) { super.printStackTrace(s); } else { // Override to simply print the first line of the trace, which is the current exception. - // Since we aren't serializing the repetitive stacktrace onShardFailureWrapper, we shouldn't print it out either + // Note: This will also omit the cause chain or any suppressed exceptions. s.println(this); } } diff --git a/server/src/main/java/org/elasticsearch/action/UnavailableShardsException.java b/server/src/main/java/org/elasticsearch/action/UnavailableShardsException.java index b4120804c20df..647e98e3599f5 100644 --- a/server/src/main/java/org/elasticsearch/action/UnavailableShardsException.java +++ b/server/src/main/java/org/elasticsearch/action/UnavailableShardsException.java @@ -45,4 +45,9 @@ public UnavailableShardsException(StreamInput in) throws IOException { public RestStatus status() { return RestStatus.SERVICE_UNAVAILABLE; } + + @Override + public Throwable fillInStackTrace() { + return this; // this exception doesn't imply a bug, no need for a stack trace + } } diff --git a/server/src/main/java/org/elasticsearch/cluster/block/ClusterBlockException.java b/server/src/main/java/org/elasticsearch/cluster/block/ClusterBlockException.java index 6e4968c8f359a..c496ccccd9c10 100644 --- a/server/src/main/java/org/elasticsearch/cluster/block/ClusterBlockException.java +++ b/server/src/main/java/org/elasticsearch/cluster/block/ClusterBlockException.java @@ -38,6 +38,11 @@ public ClusterBlockException(StreamInput in) throws IOException { this.blocks = in.readCollectionAsImmutableSet(ClusterBlock::new); } + @Override + public Throwable fillInStackTrace() { + return this; // this exception doesn't imply a bug, no need for a stack trace + } + @Override protected void writeTo(StreamOutput out, Writer nestedExceptionsWriter) throws IOException { super.writeTo(out, nestedExceptionsWriter); diff --git a/server/src/main/java/org/elasticsearch/common/breaker/CircuitBreakingException.java b/server/src/main/java/org/elasticsearch/common/breaker/CircuitBreakingException.java index 15b3269ab7db7..8e2d4d56f7686 100644 --- a/server/src/main/java/org/elasticsearch/common/breaker/CircuitBreakingException.java +++ b/server/src/main/java/org/elasticsearch/common/breaker/CircuitBreakingException.java @@ -42,6 +42,11 @@ public CircuitBreakingException(String message, long bytesWanted, long byteLimit this.durability = durability; } + @Override + public Throwable fillInStackTrace() { + return this; // this exception doesn't imply a bug, no need for a stack trace + } + @Override protected void writeTo(StreamOutput out, Writer nestedExceptionsWriter) throws IOException { super.writeTo(out, nestedExceptionsWriter); diff --git a/server/src/main/java/org/elasticsearch/common/util/concurrent/EsRejectedExecutionException.java b/server/src/main/java/org/elasticsearch/common/util/concurrent/EsRejectedExecutionException.java index 6a67f41ab8004..3a21ea486ce39 100644 --- a/server/src/main/java/org/elasticsearch/common/util/concurrent/EsRejectedExecutionException.java +++ b/server/src/main/java/org/elasticsearch/common/util/concurrent/EsRejectedExecutionException.java @@ -27,6 +27,11 @@ public EsRejectedExecutionException() { this(null, false); } + @Override + public Throwable fillInStackTrace() { + return this; // this exception doesn't imply a bug, no need for a stack trace + } + /** * Checks if the thread pool that rejected the execution was terminated * shortly after the rejection. Its possible that this returns false and the diff --git a/server/src/main/java/org/elasticsearch/discovery/MasterNotDiscoveredException.java b/server/src/main/java/org/elasticsearch/discovery/MasterNotDiscoveredException.java index 0bd62627cb061..0f982e0e0ec65 100644 --- a/server/src/main/java/org/elasticsearch/discovery/MasterNotDiscoveredException.java +++ b/server/src/main/java/org/elasticsearch/discovery/MasterNotDiscoveredException.java @@ -32,4 +32,9 @@ public RestStatus status() { public MasterNotDiscoveredException(StreamInput in) throws IOException { super(in); } + + @Override + public Throwable fillInStackTrace() { + return this; // this exception doesn't imply a bug, no need for a stack trace + } } diff --git a/server/src/main/java/org/elasticsearch/node/NodeClosedException.java b/server/src/main/java/org/elasticsearch/node/NodeClosedException.java index 4a99c9be7b78a..d2e0f71426df0 100644 --- a/server/src/main/java/org/elasticsearch/node/NodeClosedException.java +++ b/server/src/main/java/org/elasticsearch/node/NodeClosedException.java @@ -28,4 +28,9 @@ public NodeClosedException(DiscoveryNode node) { public NodeClosedException(StreamInput in) throws IOException { super(in); } + + @Override + public Throwable fillInStackTrace() { + return this; // this exception doesn't imply a bug, no need for a stack trace + } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/MultiBucketConsumerService.java b/server/src/main/java/org/elasticsearch/search/aggregations/MultiBucketConsumerService.java index a6f634ec371b1..2519e4e263d00 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/MultiBucketConsumerService.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/MultiBucketConsumerService.java @@ -65,6 +65,11 @@ public TooManyBucketsException(StreamInput in) throws IOException { maxBuckets = in.readInt(); } + @Override + public Throwable fillInStackTrace() { + return this; // this exception doesn't imply a bug, no need for a stack trace + } + @Override protected void writeTo(StreamOutput out, Writer nestedExceptionsWriter) throws IOException { super.writeTo(out, nestedExceptionsWriter); diff --git a/server/src/main/java/org/elasticsearch/transport/NodeNotConnectedException.java b/server/src/main/java/org/elasticsearch/transport/NodeNotConnectedException.java index 6e1f29353f78a..b54d6bdfe3d75 100644 --- a/server/src/main/java/org/elasticsearch/transport/NodeNotConnectedException.java +++ b/server/src/main/java/org/elasticsearch/transport/NodeNotConnectedException.java @@ -27,4 +27,9 @@ public NodeNotConnectedException(DiscoveryNode node, String msg) { public NodeNotConnectedException(StreamInput in) throws IOException { super(in); } + + @Override + public Throwable fillInStackTrace() { + return this; // this exception doesn't imply a bug, no need for a stack trace + } } From 7ef01aae8ed42cc2013a70ef689ccef7cac11579 Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Fri, 12 Jul 2024 09:40:26 +0200 Subject: [PATCH 2/2] revert CircuitBreakingException due to usage in painless --- .../common/breaker/CircuitBreakingException.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/breaker/CircuitBreakingException.java b/server/src/main/java/org/elasticsearch/common/breaker/CircuitBreakingException.java index 8e2d4d56f7686..15b3269ab7db7 100644 --- a/server/src/main/java/org/elasticsearch/common/breaker/CircuitBreakingException.java +++ b/server/src/main/java/org/elasticsearch/common/breaker/CircuitBreakingException.java @@ -42,11 +42,6 @@ public CircuitBreakingException(String message, long bytesWanted, long byteLimit this.durability = durability; } - @Override - public Throwable fillInStackTrace() { - return this; // this exception doesn't imply a bug, no need for a stack trace - } - @Override protected void writeTo(StreamOutput out, Writer nestedExceptionsWriter) throws IOException { super.writeTo(out, nestedExceptionsWriter);