-
Notifications
You must be signed in to change notification settings - Fork 588
HDDS-3498. Shutdown datanode if address is already in use #7256
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
Changes from 11 commits
7d3ccfe
c9e88a2
9fd9d99
20f4470
4d16aa7
71588eb
58741b5
6017207
29400e5
164f1c0
83fbbbe
4a46f4d
4b4a582
6253cb1
d9536ab
3bb033d
0ac758e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,7 @@ | |
| import io.opentracing.Span; | ||
| import io.opentracing.util.GlobalTracer; | ||
| import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration; | ||
| import org.apache.hadoop.ozone.container.ozoneimpl.BindException; | ||
| import org.apache.ratis.thirdparty.io.grpc.Server; | ||
| import org.apache.ratis.thirdparty.io.grpc.ServerInterceptors; | ||
| import org.apache.ratis.thirdparty.io.grpc.netty.GrpcSslContexts; | ||
|
|
@@ -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")) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(e); | ||
| } else { | ||
| throw e; | ||
| } | ||
| } | ||
| int realPort = server.getPort(); | ||
|
|
||
| if (port == 0) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.apache.hadoop.ozone.container.ozoneimpl; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
| /** | ||
| * Exception used to indicate a problem with binding a port. | ||
| * Typically, the port is in use. | ||
myskov marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| */ | ||
| public class BindException extends IOException { | ||
| public BindException() { | ||
| } | ||
|
|
||
| public BindException(String message) { | ||
| super(message); | ||
| } | ||
|
|
||
| public BindException(String message, Throwable cause) { | ||
| super(message, cause); | ||
| } | ||
|
|
||
| public BindException(Throwable cause) { | ||
| super(cause); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done