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

Remove some obsolete exceptions #69675

Merged

Conversation

DaveCTurner
Copy link
Contributor

  • NodeShouldNotConnectException has not been instantiated since 5.0
  • GatewayException has not been instantiated since 5.0
  • SnapshotFailedEngineException has not been instantated since 6.2.0
    (Primary send safe commit in file-based recovery #28038) and was never thrown across clusters

This commit removes these obsolete exceptions.

- `NodeShouldNotConnectException` has not been instantiated since 5.0
- `GatewayException` has not been instantiated since 5.0
- `SnapshotFailedEngineException` has not been instantated since 6.2.0
  (elastic#28038) and was never thrown across clusters

This commit removes these obsolete exceptions.
@DaveCTurner DaveCTurner added >non-issue :Distributed Indexing/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. v8.0.0 v7.13.0 labels Mar 1, 2021
@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Mar 1, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

Copy link
Member

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

LGTM :)

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
Member

@original-brownbear original-brownbear Mar 1, 2021

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

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 :)

@DaveCTurner DaveCTurner merged commit 91b41cd into elastic:master Mar 1, 2021
@DaveCTurner DaveCTurner deleted the 2021-03-01-remove-obsolete-exceptions branch March 1, 2021 12:14
DaveCTurner added a commit that referenced this pull request Mar 1, 2021
- `NodeShouldNotConnectException` has not been instantiated since 5.0
- `GatewayException` has not been instantiated since 5.0
- `SnapshotFailedEngineException` has not been instantated since 6.2.0
  (#28038) and was never thrown across clusters

This commit removes these obsolete exceptions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Indexing/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. >non-issue Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v7.13.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants