Skip to content
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

Skip stacktrace for exceptions when stacktrace is not relevant. #110785

Merged
merged 4 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<Throwable> nestedExceptionsWriter) throws IOException {
super.writeTo(out, nestedExceptionsWriter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Throwable> nestedExceptionsWriter) throws IOException {
super.writeTo(out, nestedExceptionsWriter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<Throwable> nestedExceptionsWriter) throws IOException {
super.writeTo(out, nestedExceptionsWriter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,9 @@ public NodeNotConnectedException(DiscoveryNode node, String msg) {
public NodeNotConnectedException(StreamInput in) throws IOException {
super(in);
}

@Override
public Throwable fillInStackTrace() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it make sense to have a common abstract 'nonprintable exception' class?

return this; // this exception doesn't imply a bug, no need for a stack trace
}
}