morningman commented on code in PR #16315:
URL: https://github.com/apache/doris/pull/16315#discussion_r1121176044
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java:
##########
@@ -354,6 +355,7 @@ public class Env {
private int masterRpcPort;
private int masterHttpPort;
private String masterIp;
+ private String masterHostName;
Review Comment:
Better merge `masterIp` and `masterHostName` into one field
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java:
##########
@@ -942,24 +945,13 @@ private void getClusterIdAndRole() throws IOException {
// But we will get a empty nodeName after upgrading.
// So for forward compatibility, we use the "old-style"
way of naming: "ip_port",
// and update the ROLE file.
- nodeName = genFeNodeName(selfNode.first, selfNode.second,
true/* old style */);
+ nodeName = genFeNodeName(selfNode.getIp(),
selfNode.getPort(), true/* old style */);
storage.writeFrontendRoleAndNodeName(role, nodeName);
LOG.info("forward compatibility. role: {}, node name: {}",
role.name(), nodeName);
- } else {
- // nodeName should be like "192.168.1.1_9217_1620296111213"
- // and the selfNode should be the prefix of nodeName.
- // If not, it means that the ip used last time is
different from this time, which is not allowed.
- // But is metadata_failure_recovery is true,
- // we will not check it because this may be a FE migration.
- String[] split = nodeName.split("_");
- if (Config.metadata_failure_recovery.equals("false") &&
!selfNode.first.equalsIgnoreCase(
- split[0])) {
- throw new IOException(
- "the self host " + selfNode.first + " does not
equal to the host in ROLE" + " file "
- + split[0] + ". You need to set
'priority_networks' config"
- + " in fe.conf to match the host " +
split[0]);
- }
}
+ // Notice:
Review Comment:
I think we should still keep this check if `Config.enable_fqdn_mode` is false
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java:
##########
@@ -1044,15 +1037,16 @@ private void getClusterIdAndRole() throws IOException {
clusterId = storage.getClusterID();
token = storage.getToken();
try {
- URL idURL = new URL("http://"; +
NetUtils.getHostPortInAccessibleFormat(rightHelperNode.first, Config.http_port)
+ "/check");
+ URL idURL = new URL("http://"; +
NetUtils.getHostPortInAccessibleFormat(rightHelperNode.getIp(),
Config.http_port) + "/check");
HttpURLConnection conn = null;
conn = (HttpURLConnection) idURL.openConnection();
conn.setConnectTimeout(2 * 1000);
conn.setReadTimeout(2 * 1000);
String clusterIdString =
conn.getHeaderField(MetaBaseAction.CLUSTER_ID);
int remoteClusterId = Integer.parseInt(clusterIdString);
if (remoteClusterId != clusterId) {
- LOG.error("cluster id is not equal with helper node
{}. will exit.", rightHelperNode.first);
+ LOG.error("cluster id is not equal with helper node
{}. will exit.",
Review Comment:
throw IOException, and the caller will do the rest
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java:
##########
@@ -1461,7 +1476,7 @@ private void transferToNonMaster(FrontendNodeType
newType) {
if (Config.edit_log_type.equalsIgnoreCase("bdb")) {
for (Frontend fe : frontends.values()) {
if (fe.getRole() == FrontendNodeType.FOLLOWER || fe.getRole()
== FrontendNodeType.REPLICA) {
- ((BDBHA) getHaProtocol()).addHelperSocket(fe.getHost(),
fe.getEditLogPort());
+ ((BDBHA) getHaProtocol()).addHelperSocket(fe.getIp(),
fe.getEditLogPort());
Review Comment:
If we add IP here, how can bdbje know that the ip is changed?
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java:
##########
@@ -1159,8 +1158,22 @@ private boolean getFeNodeTypeAndNameFromHelpers() {
return true;
}
- private void getSelfHostPort() {
- selfNode = Pair.of(FrontendOptions.getLocalHostAddress(),
Config.edit_log_port);
+ private void getSelfHostPort() throws Exception {
+ String hostName = FrontendOptions.getHostname();
+ if (hostName.equals(FrontendOptions.getLocalHostAddress())) {
+ if (Config.enable_fqdn_mode) {
+ LOG.fatal("Can't get hostname in FQDN mode. Please check your
network configuration."
Review Comment:
LOG.warn
##########
fe/fe-core/src/main/java/org/apache/doris/system/Frontend.java:
##########
@@ -131,10 +156,8 @@ public boolean handleHbResponse(FrontendHbResponse
hbResponse, boolean isReplay)
@Override
public void write(DataOutput out) throws IOException {
- Text.writeString(out, role.name());
- Text.writeString(out, host);
- out.writeInt(editLogPort);
- Text.writeString(out, nodeName);
+ String json = GsonUtils.GSON.toJson(this);
+ Text.writeString(out, json);
}
public void readFields(DataInput in) throws IOException {
Review Comment:
public -> private, and mark it as @Deprecated
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/FrontendClause.java:
##########
@@ -61,10 +66,11 @@ public void analyze(Analyzer analyzer) throws
AnalysisException {
analyzer.getQualifiedUser());
}
- Pair<String, Integer> pair =
SystemInfoService.validateHostAndPort(hostPort);
- this.host = pair.first;
- this.port = pair.second;
- Preconditions.checkState(!Strings.isNullOrEmpty(host));
+ HostInfo pair = SystemInfoService.getIpHostAndPort(hostPort, true);
Review Comment:
HostInfo hostInfo
##########
fe/fe-core/src/main/java/org/apache/doris/ha/MasterInfo.java:
##########
@@ -27,17 +29,20 @@
public class MasterInfo implements Writable {
private String ip;
+ private String hostName;
Review Comment:
And it is every error prone to be null for a field.
How about keep it as `empty string` is not set, and use `Strings.isEmpty()`
to check it when use it?
Same suggestion for host in Frontend.java
##########
fe/fe-core/src/main/java/org/apache/doris/ha/MasterInfo.java:
##########
@@ -27,17 +29,20 @@
public class MasterInfo implements Writable {
private String ip;
+ private String hostName;
Review Comment:
The hostName maybe null, we need to handle it.
And you can change the serde to GSON for this class.
##########
fe/fe-core/src/main/java/org/apache/doris/system/FQDNManager.java:
##########
@@ -40,10 +41,36 @@ public FQDNManager(SystemInfoService nodeMgr) {
}
/**
- * At each round: check if ip of be has already been changed
+ * At each round: check if ip of be or fe has already been changed
*/
@Override
protected void runAfterCatalogReady() {
+ updateBeIp();
+ updateFeIp();
+ }
+
+ private void updateFeIp() {
+ for (Frontend fe : Env.getCurrentEnv().getFrontends(null /* all */)) {
+ if (fe.getHostName() != null) {
+ try {
+ InetAddress inetAddress =
InetAddress.getByName(fe.getHostName());
+ if
(!fe.getIp().equalsIgnoreCase(inetAddress.getHostAddress())) {
+ String oldIp = fe.getIp();
+ String newIp = inetAddress.getHostAddress();
+ Env.getCurrentEnv().modifyFrontendIp(fe.getNodeName(),
newIp);
+ LOG.info("ip for {} of fe has been changed from {} to
{}",
Review Comment:
LOG.warn
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java:
##########
@@ -2452,58 +2468,103 @@ protected void runAfterCatalogReady() {
};
}
- public void addFrontend(FrontendNodeType role, String host, int
editLogPort) throws DdlException {
+ public void addFrontend(FrontendNodeType role, String ip, String hostname,
int editLogPort) throws DdlException {
if (!tryLock(false)) {
throw new DdlException("Failed to acquire catalog lock. Try
again");
}
try {
- Frontend fe = checkFeExist(host, editLogPort);
+ Frontend fe = checkFeExist(ip, hostname, editLogPort);
if (fe != null) {
throw new DdlException("frontend already exists " + fe);
}
-
+ if (Config.enable_fqdn_mode && StringUtils.isEmpty(hostname)) {
+ throw new DdlException("frontend's hostName should not be
empty while enable_fqdn_mode is true");
+ }
+ String host = hostname != null && Config.enable_fqdn_mode ?
hostname : ip;
String nodeName = genFeNodeName(host, editLogPort, false /* new
name style */);
if (removedFrontends.contains(nodeName)) {
throw new DdlException("frontend name already exists " +
nodeName + ". Try again");
}
- fe = new Frontend(role, nodeName, host, editLogPort);
+ fe = new Frontend(role, nodeName, ip, hostname, editLogPort);
frontends.put(nodeName, fe);
BDBHA bdbha = (BDBHA) haProtocol;
if (role == FrontendNodeType.FOLLOWER || role ==
FrontendNodeType.REPLICA) {
- bdbha.addHelperSocket(host, editLogPort);
- helperNodes.add(Pair.of(host, editLogPort));
+ bdbha.addHelperSocket(ip, editLogPort);
+ helperNodes.add(new HostInfo(ip, hostname, editLogPort));
bdbha.addUnReadyElectableNode(nodeName, getFollowerCount());
}
- bdbha.removeConflictNodeIfExist(host, editLogPort);
+ bdbha.removeConflictNodeIfExist(ip, editLogPort);
editLog.logAddFrontend(fe);
} finally {
unlock();
}
}
- public void dropFrontend(FrontendNodeType role, String host, int port)
throws DdlException {
- if (host.equals(selfNode.first) && port == selfNode.second && feType
== FrontendNodeType.MASTER) {
+ public void modifyFrontendIp(String nodeName, String destIp) throws
DdlException {
+ modifyFrontendHost(nodeName, destIp, null);
+ }
+
+ public void modifyFrontendHostName(String nodeName, String destHostName)
throws DdlException {
+ modifyFrontendHost(nodeName, null, destHostName);
+ }
+
+ public void modifyFrontendHost(String nodeName, String destIp, String
destHostName) throws DdlException {
+ if (!tryLock(false)) {
+ throw new DdlException("Failed to acquire catalog lock. Try
again");
+ }
+ try {
+ Frontend fe = getFeByName(nodeName);
+ if (fe == null) {
+ throw new DdlException("frontend does not exist, nodeName:" +
nodeName);
+ }
+ boolean needLog = false;
+ // we use hostname as address of bdbha, so we not need to update
node address when ip changed
+ if (destIp != null && !destIp.equals(fe.getIp())) {
+ fe.setIp(destIp);
+ needLog = true;
+ }
+ if (destHostName != null &&
!destHostName.equals(fe.getHostName())) {
+ fe.setHostName(destHostName);
+ BDBHA bdbha = (BDBHA) haProtocol;
+ bdbha.updateNodeAddress(fe.getNodeName(), destHostName,
fe.getEditLogPort());
+ needLog = true;
+ }
+ if (needLog) {
+ Env.getCurrentEnv().getEditLog().logModifyFrontend(fe);
+ }
+ } finally {
+ unlock();
+ }
+ }
+
+ public void dropFrontend(FrontendNodeType role, String ip, String
hostname, int port) throws DdlException {
+ if (port == selfNode.getPort() && feType == FrontendNodeType.MASTER
+ && ((selfNode.getHostName() != null &&
selfNode.getHostName().equals(hostname))
+ || ip.equals(selfNode.getIp()))) {
throw new DdlException("can not drop current master node.");
}
if (!tryLock(false)) {
throw new DdlException("Failed to acquire catalog lock. Try
again");
}
try {
- Frontend fe = checkFeExist(host, port);
+ Frontend fe = checkFeExist(ip, hostname, port);
if (fe == null) {
- throw new DdlException("frontend does not exist[" + host + ":"
+ port + "]");
+ throw new DdlException("frontend does not exist[" + ip + ":" +
port + "]");
}
if (fe.getRole() != role) {
- throw new DdlException(role.toString() + " does not exist[" +
host + ":" + port + "]");
+ throw new DdlException(role.toString() + " does not exist[" +
ip + ":" + port + "]");
}
frontends.remove(fe.getNodeName());
removedFrontends.add(fe.getNodeName());
if (fe.getRole() == FrontendNodeType.FOLLOWER || fe.getRole() ==
FrontendNodeType.REPLICA) {
haProtocol.removeElectableNode(fe.getNodeName());
- helperNodes.remove(Pair.of(host, port));
+ // ip may be changed, so we need use both ip and hostname to
check.
Review Comment:
Duplicate code with line 3335, extract to a method
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java:
##########
@@ -1159,8 +1158,22 @@ private boolean getFeNodeTypeAndNameFromHelpers() {
return true;
}
- private void getSelfHostPort() {
- selfNode = Pair.of(FrontendOptions.getLocalHostAddress(),
Config.edit_log_port);
+ private void getSelfHostPort() throws Exception {
+ String hostName = FrontendOptions.getHostname();
+ if (hostName.equals(FrontendOptions.getLocalHostAddress())) {
+ if (Config.enable_fqdn_mode) {
+ LOG.fatal("Can't get hostname in FQDN mode. Please check your
network configuration."
Review Comment:
And this logic is strange, I think it can be done inside the
`FrontendOptions` class, because all info is got from `FrontendOptions`
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java:
##########
@@ -2452,58 +2468,103 @@ protected void runAfterCatalogReady() {
};
}
- public void addFrontend(FrontendNodeType role, String host, int
editLogPort) throws DdlException {
+ public void addFrontend(FrontendNodeType role, String ip, String hostname,
int editLogPort) throws DdlException {
if (!tryLock(false)) {
throw new DdlException("Failed to acquire catalog lock. Try
again");
}
try {
- Frontend fe = checkFeExist(host, editLogPort);
+ Frontend fe = checkFeExist(ip, hostname, editLogPort);
if (fe != null) {
throw new DdlException("frontend already exists " + fe);
}
-
+ if (Config.enable_fqdn_mode && StringUtils.isEmpty(hostname)) {
+ throw new DdlException("frontend's hostName should not be
empty while enable_fqdn_mode is true");
+ }
+ String host = hostname != null && Config.enable_fqdn_mode ?
hostname : ip;
String nodeName = genFeNodeName(host, editLogPort, false /* new
name style */);
if (removedFrontends.contains(nodeName)) {
throw new DdlException("frontend name already exists " +
nodeName + ". Try again");
}
- fe = new Frontend(role, nodeName, host, editLogPort);
+ fe = new Frontend(role, nodeName, ip, hostname, editLogPort);
frontends.put(nodeName, fe);
BDBHA bdbha = (BDBHA) haProtocol;
if (role == FrontendNodeType.FOLLOWER || role ==
FrontendNodeType.REPLICA) {
- bdbha.addHelperSocket(host, editLogPort);
- helperNodes.add(Pair.of(host, editLogPort));
+ bdbha.addHelperSocket(ip, editLogPort);
+ helperNodes.add(new HostInfo(ip, hostname, editLogPort));
bdbha.addUnReadyElectableNode(nodeName, getFollowerCount());
}
- bdbha.removeConflictNodeIfExist(host, editLogPort);
+ bdbha.removeConflictNodeIfExist(ip, editLogPort);
editLog.logAddFrontend(fe);
} finally {
unlock();
}
}
- public void dropFrontend(FrontendNodeType role, String host, int port)
throws DdlException {
- if (host.equals(selfNode.first) && port == selfNode.second && feType
== FrontendNodeType.MASTER) {
+ public void modifyFrontendIp(String nodeName, String destIp) throws
DdlException {
+ modifyFrontendHost(nodeName, destIp, null);
+ }
+
+ public void modifyFrontendHostName(String nodeName, String destHostName)
throws DdlException {
+ modifyFrontendHost(nodeName, null, destHostName);
+ }
+
+ public void modifyFrontendHost(String nodeName, String destIp, String
destHostName) throws DdlException {
+ if (!tryLock(false)) {
+ throw new DdlException("Failed to acquire catalog lock. Try
again");
+ }
+ try {
+ Frontend fe = getFeByName(nodeName);
+ if (fe == null) {
+ throw new DdlException("frontend does not exist, nodeName:" +
nodeName);
+ }
+ boolean needLog = false;
+ // we use hostname as address of bdbha, so we not need to update
node address when ip changed
Review Comment:
we not -> we don't
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
