Skip to content

Remove some obsolete exceptions#69675

Merged
DaveCTurner merged 2 commits intoelastic:masterfrom
DaveCTurner:2021-03-01-remove-obsolete-exceptions
Mar 1, 2021
Merged

Remove some obsolete exceptions#69675
DaveCTurner merged 2 commits intoelastic:masterfrom
DaveCTurner:2021-03-01-remove-obsolete-exceptions

Conversation

@DaveCTurner
Copy link
Copy Markdown
Member

  • 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/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 Meta label for distributed team. label Mar 1, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

Copy link
Copy Markdown
Contributor

@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
Copy Markdown
Contributor

@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
Copy Markdown
Member 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/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. >non-issue Team:Distributed Meta label for distributed team. v7.13.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants