Skip to content

Remove some obsolete exceptions #69675

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 1, 2021
Merged

Remove some obsolete exceptions #69675

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d165c02
Remove some obsolete exceptions
DaveCTurner Mar 1, 2021
1b5cf94
Fix comments
DaveCTurner Mar 1, 2021
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
9 changes: 3 additions & 6 deletions server/src/main/java/org/elasticsearch/ElasticsearchException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,8 +741,7 @@ private enum ElasticsearchExceptionHandle {
org.elasticsearch.action.search.ReduceSearchPhaseException::new, 8, UNKNOWN_VERSION_ADDED),
NODE_CLOSED_EXCEPTION(org.elasticsearch.node.NodeClosedException.class,
org.elasticsearch.node.NodeClosedException::new, 9, UNKNOWN_VERSION_ADDED),
SNAPSHOT_FAILED_ENGINE_EXCEPTION(org.elasticsearch.index.engine.SnapshotFailedEngineException.class,
org.elasticsearch.index.engine.SnapshotFailedEngineException::new, 10, UNKNOWN_VERSION_ADDED),
// 10 was for SnapshotFailedEngineException, never instantiated in 6.0+ and never thrown across clusters
SHARD_NOT_FOUND_EXCEPTION(org.elasticsearch.index.shard.ShardNotFoundException.class,
org.elasticsearch.index.shard.ShardNotFoundException::new, 11, UNKNOWN_VERSION_ADDED),
CONNECT_TRANSPORT_EXCEPTION(org.elasticsearch.transport.ConnectTransportException.class,
Expand Down Expand Up @@ -810,8 +809,7 @@ private enum ElasticsearchExceptionHandle {
org.elasticsearch.indices.recovery.RecoveryFailedException::new, 44, UNKNOWN_VERSION_ADDED),
INDEX_SHARD_RELOCATED_EXCEPTION(org.elasticsearch.index.shard.IndexShardRelocatedException.class,
org.elasticsearch.index.shard.IndexShardRelocatedException::new, 45, UNKNOWN_VERSION_ADDED),
NODE_SHOULD_NOT_CONNECT_EXCEPTION(org.elasticsearch.transport.NodeShouldNotConnectException.class,
org.elasticsearch.transport.NodeShouldNotConnectException::new, 46, UNKNOWN_VERSION_ADDED),
// 46 was for NodeShouldNotConnectException, never instantiated in 5.0+
// 47 used to be for IndexTemplateAlreadyExistsException which was deprecated in 5.1 removed in 6.0
TRANSLOG_CORRUPTED_EXCEPTION(org.elasticsearch.index.translog.TranslogCorruptedException.class,
org.elasticsearch.index.translog.TranslogCorruptedException::new, 48, UNKNOWN_VERSION_ADDED),
Expand Down Expand Up @@ -841,8 +839,7 @@ private enum ElasticsearchExceptionHandle {
ALIAS_FILTER_PARSING_EXCEPTION(org.elasticsearch.indices.AliasFilterParsingException.class,
org.elasticsearch.indices.AliasFilterParsingException::new, 63, UNKNOWN_VERSION_ADDED),
// 64 was DeleteByQueryFailedEngineException, which was removed in 5.0
GATEWAY_EXCEPTION(org.elasticsearch.gateway.GatewayException.class, org.elasticsearch.gateway.GatewayException::new, 65,
UNKNOWN_VERSION_ADDED),
// 65 was for GatewayException, never instantiated in 5.0+
INDEX_SHARD_NOT_RECOVERING_EXCEPTION(org.elasticsearch.index.shard.IndexShardNotRecoveringException.class,
org.elasticsearch.index.shard.IndexShardNotRecoveringException::new, 66, UNKNOWN_VERSION_ADDED),
HTTP_EXCEPTION(org.elasticsearch.http.HttpException.class, org.elasticsearch.http.HttpException::new, 67, UNKNOWN_VERSION_ADDED),
Expand Down
5 changes: 1 addition & 4 deletions .../java/org/elasticsearch/action/support/broadcast/node/TransportBroadcastByNodeAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import org.elasticsearch.tasks.Task;
import org.elasticsearch.tasks.TaskCancelledException;
import org.elasticsearch.tasks.TaskId;
import org.elasticsearch.transport.NodeShouldNotConnectException;
import org.elasticsearch.transport.TransportChannel;
import org.elasticsearch.transport.TransportException;
import org.elasticsearch.transport.TransportRequest;
Expand Down Expand Up @@ -360,9 +359,7 @@ protected void onNodeResponse(DiscoveryNode node, int nodeIndex, NodeResponse re

protected void onNodeFailure(DiscoveryNode node, int nodeIndex, Throwable t) {
String nodeId = node.getId();
if (logger.isDebugEnabled() && (t instanceof NodeShouldNotConnectException) == false) {
logger.debug(new ParameterizedMessage("failed to execute [{}] on node [{}]", actionName, nodeId), t);
}
logger.debug(new ParameterizedMessage("failed to execute [{}] on node [{}]", actionName, nodeId), t);
Copy link
Contributor

@original-brownbear original-brownbear Mar 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

NIT: maybe keep the logger.isDebugEnabled() here and in the other 2 spots changed so that we don't have to create the message object needlessly? (doesn't look too performance critical ever :) so pretty optional)

Copy link
Contributor Author

@DaveCTurner DaveCTurner Mar 1, 2021

Choose a reason for hiding this comment

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

I'd rather not, we're on a (hopefully rare) failure path here already, and making this one extra object should be super-cheap whereas making me read an extra conditional every time I look at this is not :)


// this is defensive to protect against the possibility of double invocation
// the current implementation of TransportService#sendRequest guards against this
Expand Down
5 changes: 1 addition & 4 deletions server/src/main/java/org/elasticsearch/action/support/nodes/TransportNodesAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.NodeShouldNotConnectException;
import org.elasticsearch.transport.TransportChannel;
import org.elasticsearch.transport.TransportException;
import org.elasticsearch.transport.TransportRequest;
Expand Down Expand Up @@ -247,9 +246,7 @@ private void onOperation(int idx, NodeResponse nodeResponse) {
}

private void onFailure(int idx, String nodeId, Throwable t) {
if (logger.isDebugEnabled() && (t instanceof NodeShouldNotConnectException) == false) {
logger.debug(new ParameterizedMessage("failed to execute on node [{}]", nodeId), t);
}
logger.debug(new ParameterizedMessage("failed to execute on node [{}]", nodeId), t);
responses.set(idx, new FailedNodeException(nodeId, "Failed node [" + nodeId + "]", t));
if (counter.incrementAndGet() == responses.length()) {
finishHim();
Expand Down
5 changes: 1 addition & 4 deletions server/src/main/java/org/elasticsearch/action/support/tasks/TransportTasksAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.util.concurrent.AtomicArray;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.transport.NodeShouldNotConnectException;
import org.elasticsearch.transport.TransportChannel;
import org.elasticsearch.transport.TransportException;
import org.elasticsearch.transport.TransportRequest;
Expand Down Expand Up @@ -279,9 +278,7 @@ private void onOperation(int idx, NodeTasksResponse nodeResponse) {
}

private void onFailure(int idx, String nodeId, Throwable t) {
if (logger.isDebugEnabled() && (t instanceof NodeShouldNotConnectException) == false) {
logger.debug(new ParameterizedMessage("failed to execute on node [{}]", nodeId), t);
}
logger.debug(new ParameterizedMessage("failed to execute on node [{}]", nodeId), t);

responses.set(idx, new FailedNodeException(nodeId, "Failed node [" + nodeId + "]", t));

Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/gateway/Gateway.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public Gateway(final ClusterService clusterService, final NodeClient client) {
this.client = client;
}

public void performStateRecovery(final GatewayStateRecoveredListener listener) throws GatewayException {
public void performStateRecovery(final GatewayStateRecoveredListener listener) {
final String[] nodesIds = clusterService.state().nodes().getMasterNodes().keys().toArray(String.class);
logger.trace("performing state recovery from {}", Arrays.toString(nodesIds));
var request = new TransportNodesListGatewayMetaState.Request(nodesIds);
Expand Down
21 changes: 0 additions & 21 deletions server/src/main/java/org/elasticsearch/gateway/GatewayException.java
View file
Open in desktop

This file was deleted.

20 changes: 0 additions & 20 deletions server/src/main/java/org/elasticsearch/index/engine/SnapshotFailedEngineException.java
View file
Open in desktop

This file was deleted.

20 changes: 0 additions & 20 deletions server/src/main/java/org/elasticsearch/transport/NodeShouldNotConnectException.java
View file
Open in desktop

This file was deleted.

6 changes: 3 additions & 3 deletions server/src/test/java/org/elasticsearch/ExceptionSerializationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ public void testIds() {
ids.put(7, org.elasticsearch.http.BindHttpException.class);
ids.put(8, org.elasticsearch.action.search.ReduceSearchPhaseException.class);
ids.put(9, org.elasticsearch.node.NodeClosedException.class);
ids.put(10, org.elasticsearch.index.engine.SnapshotFailedEngineException.class);
ids.put(10, null); // SnapshotFailedEngineException, never instantiated in 6.0+ and never thrown across clusters
ids.put(11, org.elasticsearch.index.shard.ShardNotFoundException.class);
ids.put(12, org.elasticsearch.transport.ConnectTransportException.class);
ids.put(13, org.elasticsearch.transport.NotSerializableTransportException.class);
Expand Down Expand Up @@ -709,7 +709,7 @@ public void testIds() {
ids.put(43, org.elasticsearch.index.translog.TruncatedTranslogException.class);
ids.put(44, org.elasticsearch.indices.recovery.RecoveryFailedException.class);
ids.put(45, org.elasticsearch.index.shard.IndexShardRelocatedException.class);
ids.put(46, org.elasticsearch.transport.NodeShouldNotConnectException.class);
ids.put(46, null); // NodeShouldNotConnectException, never instantiated in 5.0+
ids.put(47, null);
ids.put(48, org.elasticsearch.index.translog.TranslogCorruptedException.class);
ids.put(49, org.elasticsearch.cluster.block.ClusterBlockException.class);
Expand All @@ -728,7 +728,7 @@ public void testIds() {
ids.put(62, org.elasticsearch.common.io.stream.NotSerializableExceptionWrapper.class);
ids.put(63, org.elasticsearch.indices.AliasFilterParsingException.class);
ids.put(64, null); // DeleteByQueryFailedEngineException was removed in 3.0
ids.put(65, org.elasticsearch.gateway.GatewayException.class);
ids.put(65, null); // GatewayException, never instantiated in 5.0+
ids.put(66, org.elasticsearch.index.shard.IndexShardNotRecoveringException.class);
ids.put(67, org.elasticsearch.http.HttpException.class);
ids.put(68, org.elasticsearch.ElasticsearchException.class);
Expand Down