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.

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

HDDS-3498. Shutdown datanode if address is already in use #7256

Merged
merged 17 commits into from
Oct 4, 2024
Merged

HDDS-3498. Shutdown datanode if address is already in use #7256

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
7d3ccfe
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 2, 2024
c9e88a2
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 2, 2024
9fd9d99
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 2, 2024
20f4470
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 2, 2024
4d16aa7
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 2, 2024
71588eb
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 2, 2024
58741b5
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 2, 2024
6017207
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 2, 2024
29400e5
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 2, 2024
164f1c0
Merge pull request #2 from Daniilchik/HDDS-3498-1
Daniilchik Oct 2, 2024
83fbbbe
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 2, 2024
4a46f4d
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 3, 2024
4b4a582
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 3, 2024
6253cb1
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 3, 2024
d9536ab
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 3, 2024
3bb033d
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 4, 2024
0ac758e
HDDS-3498. Shutdown datanode if address is already in use
Daniilchik Oct 4, 2024
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
17 changes: 11 additions & 6 deletions ...main/java/org/apache/hadoop/ozone/container/common/statemachine/EndpointStateMachine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,12 +234,17 @@ public void logIfNeeded(Exception ex) {
}

if (missCounter == 0) {
LOG.warn(
"Unable to communicate to {} server at {} for past {} seconds.",
serverName,
getAddress().getHostString() + ":" + getAddress().getPort(),
TimeUnit.MILLISECONDS.toSeconds(this.getMissedCount() *
getScmHeartbeatInterval(this.conf)), ex);
long missedDurationSeconds = TimeUnit.MILLISECONDS.toSeconds(
this.getMissedCount() * getScmHeartbeatInterval(this.conf)
);
LOG.error(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why you changed it to error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the remark. I’ll change it back. I initially changed it at the beginning of working on the task.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

"Unable to communicate to {} server at {}:{} for past {} seconds.",
serverName,
address.getAddress(),
address.getPort(),
missedDurationSeconds,
ex
);
}

if (LOG.isTraceEnabled()) {
Expand Down
3 changes: 2 additions & 1 deletion ...in/java/org/apache/hadoop/ozone/container/common/states/endpoint/VersionEndpointTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.apache.hadoop.ozone.container.common.states.endpoint;

import java.io.IOException;
import java.net.BindException;
import java.util.concurrent.Callable;

import org.apache.hadoop.hdds.conf.ConfigurationSource;
Expand Down Expand Up @@ -104,7 +105,7 @@ public EndpointStateMachine.EndPointStates call() throws Exception {
LOG.debug("Cannot execute GetVersion task as endpoint state machine " +
"is in {} state", rpcEndPoint.getState());
}
} catch (DiskOutOfSpaceException ex) {
} catch (DiskOutOfSpaceException | BindException ex) {
rpcEndPoint.setState(EndpointStateMachine.EndPointStates.SHUTDOWN);
} catch (IOException ex) {
rpcEndPoint.logIfNeeded(ex);
Expand Down
12 changes: 11 additions & 1 deletion ...ain/java/org/apache/hadoop/ozone/container/common/transport/server/XceiverServerGrpc.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.apache.hadoop.ozone.container.common.transport.server;

import java.io.IOException;
import java.net.BindException;
import java.util.Collections;
import java.util.List;
import java.util.UUID;
Expand Down Expand Up @@ -185,7 +186,16 @@ public HddsProtos.ReplicationType getServerType() {
@Override
public void start() throws IOException {
if (!isStarted) {
server.start();
try {
server.start();
} catch (IOException e) {
LOG.error("Error while starting the server", e);
if (e.getMessage().contains("Failed to bind to address")) {
Copy link
Contributor

@ivanzlenko ivanzlenko Oct 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Do we have any actual reason to start a service if we can't start a server? Shouldn't we just handle all IOException like that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we will handle all IOException like that we will end up with shutting down datanodes due to any network failures.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we identify how actual retryable exceptions looks like? Is netty throwing specifically only IOExceptions here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually it is better just handle correctly server.start part and shutdown everything here cause we can't start server. There is no point to live afterwards.

Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed it with Daniil - we need to return to this code at some point in time to refactor it.

throw new BindException();
myskov marked this conversation as resolved.
Show resolved Hide resolved
} else {
throw e;
}
}
int realPort = server.getPort();

if (port == 0) {
Expand Down