You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/04/22 13:01:22 UTC
[GitHub] [incubator-doris] caiconghui opened a new pull request, #9172: [enhancement](k8s) Support k8s_container_drift_mode for be and broker in k8s enviroment
caiconghui opened a new pull request, #9172:
URL: https://github.com/apache/incubator-doris/pull/9172
# Proposed changes
Issue Number: close #9171
## Problem Summary:
Describe the overview of changes.
## Checklist(Required)
1. Does it affect the original behavior: (Yes/No/I Don't know)
2. Has unit tests been added: (Yes/No/No Need)
3. Has document been added or modified: (Yes/No/No Need)
4. Does it need to update dependencies: (Yes/No)
5. Are there any changes that cannot be rolled back: (Yes/No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] caiconghui commented on a diff in pull request #9172: [enhancement](k8s) Support fqdn mode for be in k8s enviroment
Posted by GitBox <gi...@apache.org>.
caiconghui commented on code in PR #9172:
URL: https://github.com/apache/doris/pull/9172#discussion_r1032899046
##########
fe/fe-core/src/main/java/org/apache/doris/system/SystemInfoService.java:
##########
@@ -901,7 +915,8 @@ public void clear() {
this.idToReportVersionRef = null;
}
- public static Pair<String, Integer> validateHostAndPort(String hostPort) throws AnalysisException {
+ public static Triple<String, String, Integer> getIpHostAndPort(String hostPort, boolean strictCheck)
Review Comment:
because when pod is deconstructed，the ip of pod cannot be obtained，here is to prevent that we cannot drop backend
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yiguolei commented on a diff in pull request #9172: [enhancement](k8s) Support k8s_container_detect_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#discussion_r861552190
##########
fe/fe-core/src/main/java/org/apache/doris/system/FQDNManager.java:
##########
@@ -0,0 +1,103 @@
+// 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.doris.system;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import org.apache.doris.catalog.BrokerMgr;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.FsBroker;
+import org.apache.doris.common.FeConstants;
+import org.apache.doris.common.util.MasterDaemon;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.List;
+import java.util.Map;
+
+public class FQDNManager extends MasterDaemon {
+ private static final Logger LOG = LogManager.getLogger(FQDNManager.class);
+
+ private SystemInfoService nodeMgr;
+
+ public FQDNManager(SystemInfoService nodeMgr) {
+ super("FQDN mgr", FeConstants.ip_check_interval_second * 1000L);
+ this.nodeMgr = nodeMgr;
+ }
+
+ /**
+ * At each round: check if ip of be and broker has already been changed
+ */
+ @Override
+ protected void runAfterCatalogReady() {
+ for (Backend be : nodeMgr.getIdToBackend().values()) {
+ if (be.getHostName() != null) {
+ try {
+ InetAddress inetAddress = InetAddress.getByName(be.getHostName());
+ if (!be.getHost().equalsIgnoreCase(inetAddress.getHostAddress())) {
+ String ip = be.getHost();
+ be.setHost(inetAddress.getHostAddress());
+ Catalog.getCurrentCatalog().getEditLog().logBackendStateChange(be);
+ LOG.warn("ip for {} of be has been changed from {} to {}", be.getHostName(), ip, be.getHost());
+ }
+ } catch (UnknownHostException e) {
+ LOG.warn("unknown host name for be, {}", be.getHostName(), e);
+ if (!be.isAlive()) {
+ String ip = be.getHost();
+ be.setHost("unknown");
+ Catalog.getCurrentCatalog().getEditLog().logBackendStateChange(be);
+ LOG.warn("ip for {} of be has been changed from {} to {}", be.getHostName(), ip, "unknown");
+ }
+ }
+ }
+ }
+
+ Map<String, List<FsBroker>> brokerMap = Maps.newHashMap(
+ Catalog.getCurrentCatalog().getBrokerMgr().getBrokerListMap());
+ for (Map.Entry<String, List<FsBroker>> entry : brokerMap.entrySet()) {
+ for (FsBroker broker : entry.getValue()) {
+ if (broker.hostName != null) {
+ try {
+ InetAddress inetAddress = InetAddress.getByName(broker.hostName);
+ if (!broker.ip.equalsIgnoreCase(inetAddress.getHostAddress())) {
Review Comment:
And here.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman commented on a diff in pull request #9172: [enhancement](k8s) Support fqdn mode for be in k8s enviroment
Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #9172:
URL: https://github.com/apache/doris/pull/9172#discussion_r1032897749
##########
fe/fe-core/src/main/java/org/apache/doris/system/SystemInfoService.java:
##########
@@ -901,7 +915,8 @@ public void clear() {
this.idToReportVersionRef = null;
}
- public static Pair<String, Integer> validateHostAndPort(String hostPort) throws AnalysisException {
+ public static Triple<String, String, Integer> getIpHostAndPort(String hostPort, boolean strictCheck)
Review Comment:
why not name `strictCheck` to `isFqdnMode`?
##########
fe/fe-core/src/main/java/org/apache/doris/system/FQDNManager.java:
##########
@@ -0,0 +1,73 @@
+// 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.doris.system;
+
+import org.apache.doris.catalog.Env;
+import org.apache.doris.common.ClientPool;
+import org.apache.doris.common.FeConstants;
+import org.apache.doris.common.util.MasterDaemon;
+import org.apache.doris.thrift.TNetworkAddress;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+public class FQDNManager extends MasterDaemon {
+ private static final Logger LOG = LogManager.getLogger(FQDNManager.class);
+
+ private SystemInfoService nodeMgr;
+
+ public FQDNManager(SystemInfoService nodeMgr) {
+ super("FQDN mgr", FeConstants.ip_check_interval_second * 1000L);
+ this.nodeMgr = nodeMgr;
+ }
+
+ /**
+ * At each round: check if ip of be has already been changed
+ */
+ @Override
+ protected void runAfterCatalogReady() {
+ for (Backend be : nodeMgr.getIdToBackend().values()) {
+ if (be.getHostName() != null) {
+ try {
+ InetAddress inetAddress = InetAddress.getByName(be.getHostName());
+ if (!be.getHost().equalsIgnoreCase(inetAddress.getHostAddress())) {
+ String ip = be.getHost();
+ if (!ip.equalsIgnoreCase("unknown")) {
+ ClientPool.backendPool.clearPool(new TNetworkAddress(ip, be.getBePort()));
Review Comment:
I think we should change BE'ip and write edit log before clearing the client pool.
##########
fe/fe-core/src/main/java/org/apache/doris/system/FQDNManager.java:
##########
@@ -0,0 +1,73 @@
+// 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.doris.system;
+
+import org.apache.doris.catalog.Env;
+import org.apache.doris.common.ClientPool;
+import org.apache.doris.common.FeConstants;
+import org.apache.doris.common.util.MasterDaemon;
+import org.apache.doris.thrift.TNetworkAddress;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+public class FQDNManager extends MasterDaemon {
+ private static final Logger LOG = LogManager.getLogger(FQDNManager.class);
+
+ private SystemInfoService nodeMgr;
+
+ public FQDNManager(SystemInfoService nodeMgr) {
+ super("FQDN mgr", FeConstants.ip_check_interval_second * 1000L);
+ this.nodeMgr = nodeMgr;
+ }
+
+ /**
+ * At each round: check if ip of be has already been changed
+ */
+ @Override
+ protected void runAfterCatalogReady() {
+ for (Backend be : nodeMgr.getIdToBackend().values()) {
+ if (be.getHostName() != null) {
+ try {
+ InetAddress inetAddress = InetAddress.getByName(be.getHostName());
+ if (!be.getHost().equalsIgnoreCase(inetAddress.getHostAddress())) {
+ String ip = be.getHost();
+ if (!ip.equalsIgnoreCase("unknown")) {
+ ClientPool.backendPool.clearPool(new TNetworkAddress(ip, be.getBePort()));
Review Comment:
And maybe we should extract a method called: `backend.onIPChanged()`.
So that we can support `alter backend modify ip` later.
##########
fe/fe-core/src/main/java/org/apache/doris/system/FQDNManager.java:
##########
@@ -0,0 +1,73 @@
+// 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.doris.system;
+
+import org.apache.doris.catalog.Env;
+import org.apache.doris.common.ClientPool;
+import org.apache.doris.common.FeConstants;
+import org.apache.doris.common.util.MasterDaemon;
+import org.apache.doris.thrift.TNetworkAddress;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+public class FQDNManager extends MasterDaemon {
+ private static final Logger LOG = LogManager.getLogger(FQDNManager.class);
+
+ private SystemInfoService nodeMgr;
+
+ public FQDNManager(SystemInfoService nodeMgr) {
+ super("FQDN mgr", FeConstants.ip_check_interval_second * 1000L);
+ this.nodeMgr = nodeMgr;
+ }
+
+ /**
+ * At each round: check if ip of be has already been changed
+ */
+ @Override
+ protected void runAfterCatalogReady() {
+ for (Backend be : nodeMgr.getIdToBackend().values()) {
+ if (be.getHostName() != null) {
+ try {
+ InetAddress inetAddress = InetAddress.getByName(be.getHostName());
+ if (!be.getHost().equalsIgnoreCase(inetAddress.getHostAddress())) {
+ String ip = be.getHost();
+ if (!ip.equalsIgnoreCase("unknown")) {
+ ClientPool.backendPool.clearPool(new TNetworkAddress(ip, be.getBePort()));
+ }
+ be.setHost(inetAddress.getHostAddress());
+ Env.getCurrentEnv().getEditLog().logBackendStateChange(be);
+ LOG.warn("ip for {} of be has been changed from {} to {}", be.getHostName(), ip, be.getHost());
+ }
+ } catch (UnknownHostException e) {
+ LOG.warn("unknown host name for be, {}", be.getHostName(), e);
+ if (!be.isAlive() && !be.getHost().equalsIgnoreCase("unknown")) {
Review Comment:
Why judge `!be.isAlive()`?
Add some comment directly in code.
##########
fe/fe-core/src/main/java/org/apache/doris/system/FQDNManager.java:
##########
@@ -0,0 +1,73 @@
+// 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.doris.system;
+
+import org.apache.doris.catalog.Env;
+import org.apache.doris.common.ClientPool;
+import org.apache.doris.common.FeConstants;
+import org.apache.doris.common.util.MasterDaemon;
+import org.apache.doris.thrift.TNetworkAddress;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+public class FQDNManager extends MasterDaemon {
+ private static final Logger LOG = LogManager.getLogger(FQDNManager.class);
+
+ private SystemInfoService nodeMgr;
+
+ public FQDNManager(SystemInfoService nodeMgr) {
+ super("FQDN mgr", FeConstants.ip_check_interval_second * 1000L);
+ this.nodeMgr = nodeMgr;
+ }
+
+ /**
+ * At each round: check if ip of be has already been changed
+ */
+ @Override
+ protected void runAfterCatalogReady() {
+ for (Backend be : nodeMgr.getIdToBackend().values()) {
+ if (be.getHostName() != null) {
+ try {
+ InetAddress inetAddress = InetAddress.getByName(be.getHostName());
+ if (!be.getHost().equalsIgnoreCase(inetAddress.getHostAddress())) {
+ String ip = be.getHost();
+ if (!ip.equalsIgnoreCase("unknown")) {
+ ClientPool.backendPool.clearPool(new TNetworkAddress(ip, be.getBePort()));
+ }
+ be.setHost(inetAddress.getHostAddress());
+ Env.getCurrentEnv().getEditLog().logBackendStateChange(be);
+ LOG.warn("ip for {} of be has been changed from {} to {}", be.getHostName(), ip, be.getHost());
+ }
+ } catch (UnknownHostException e) {
+ LOG.warn("unknown host name for be, {}", be.getHostName(), e);
+ if (!be.isAlive() && !be.getHost().equalsIgnoreCase("unknown")) {
+ String ip = be.getHost();
+ ClientPool.backendPool.clearPool(new TNetworkAddress(ip, be.getBePort()));
+ be.setHost("unknown");
Review Comment:
define this `"unknown"` somewhere.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yiguolei commented on a diff in pull request #9172: [enhancement](k8s) Support k8s_container_detect_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#discussion_r861576458
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/BrokerMgr.java:
##########
@@ -273,7 +273,7 @@ public void replayDropBrokers(String name, List<FsBroker> addresses) {
try {
ArrayListMultimap<String, FsBroker> brokerAddrsMap = brokersMap.get(name);
for (FsBroker addr : addresses) {
- brokerAddrsMap.remove(addr.ip, addr);
+ brokerAddrsMap.remove(addr.hostName == null ? addr.ip : addr.hostName, addr);
Review Comment:
we should always use ip:port to delete broker.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on pull request #9172: [enhancement](k8s) Support k8s_container_detect_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
morningman commented on PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#issuecomment-1114194874
This PR needs some more detailed description:
1. Background motivation for the issue
2. Detail Solutions
3. Which interfaces and behaviors are affected
4. Documentation corresponding to the function
1. What scenarios does this function apply to
2. what problems does it solve
3. whether there are any precautions.
5. Unit test
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] stalary commented on a diff in pull request #9172: [enhancement](k8s) Support k8s_container_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
stalary commented on code in PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#discussion_r858723881
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SystemHandler.java:
##########
@@ -189,15 +188,15 @@ private List<Backend> checkDecommission(DecommissionBackendClause decommissionBa
* 2. after decommission, the remaining backend num should meet the replication num.
* 3. after decommission, The remaining space capacity can store data on decommissioned backends.
*/
- public static List<Backend> checkDecommission(List<Pair<String, Integer>> hostPortPairs)
+ public static List<Backend> checkDecommission(List<Triple<String, String, Integer>> ipHostPortTriples)
throws DdlException {
SystemInfoService infoService = Catalog.getCurrentSystemInfo();
List<Backend> decommissionBackends = Lists.newArrayList();
// check if exist
- for (Pair<String, Integer> pair : hostPortPairs) {
- Backend backend = infoService.getBackendWithHeartbeatPort(pair.first, pair.second);
+ for (Triple<String, String, Integer> triple : ipHostPortTriples) {
+ Backend backend = infoService.getBackendWithHeartbeatPort(triple.getLeft(), triple.getMiddle(), triple.getRight());
if (backend == null) {
- throw new DdlException("Backend does not exist[" + pair.first + ":" + pair.second + "]");
+ throw new DdlException("Backend does not exist[" + triple.getLeft() + ":" + triple.getLeft() + "]");
Review Comment:
`triple.getLeft()` repeat?
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] stalary commented on pull request #9172: [enhancement](k8s) Support k8s_container_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
stalary commented on PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#issuecomment-1109819853
`triple.getMiddle() == null ? triple.getLeft() : triple.getMiddle()`
This code appears many times, can we do it in the generated place?
`return Triple.of(ip, Config.enable_k8s_container_drift_mode ? hostName : ip, heartbeatPort)` whether it can be satisfied?
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] stalary commented on a diff in pull request #9172: [enhancement](k8s) Support k8s_container_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
stalary commented on code in PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#discussion_r858758597
##########
fe/fe-core/src/main/java/org/apache/doris/system/SystemInfoService.java:
##########
@@ -1061,7 +1084,7 @@ public static Pair<String, Integer> validateHostAndPort(String hostPort) throws
throw new AnalysisException("Port is out of range: " + heartbeatPort);
}
- return new Pair<String, Integer>(host, heartbeatPort);
+ return Triple.of(ip, Config.enable_k8s_container_drift_mode ? hostName : null, heartbeatPort);
Review Comment:
I mean is it possible to change this to`return Triple.of(ip, Config.enable_k8s_container_drift_mode ? hostName : ip, heartbeatPort);`
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] hello-stephen commented on pull request #9172: [enhancement](k8s) Support fqdn mode for be in k8s enviroment
Posted by GitBox <gi...@apache.org>.
hello-stephen commented on PR #9172:
URL: https://github.com/apache/doris/pull/9172#issuecomment-1328070682
TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 35.24 seconds
load time: 447 seconds
storage size: 17122714230 Bytes
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20221126155700_clickbench_pr_53507.html
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a diff in pull request #9172: [enhancement](k8s) Support k8s_container_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
caiconghui commented on code in PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#discussion_r858732136
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SystemHandler.java:
##########
@@ -189,15 +188,15 @@ private List<Backend> checkDecommission(DecommissionBackendClause decommissionBa
* 2. after decommission, the remaining backend num should meet the replication num.
* 3. after decommission, The remaining space capacity can store data on decommissioned backends.
*/
- public static List<Backend> checkDecommission(List<Pair<String, Integer>> hostPortPairs)
+ public static List<Backend> checkDecommission(List<Triple<String, String, Integer>> ipHostPortTriples)
throws DdlException {
SystemInfoService infoService = Catalog.getCurrentSystemInfo();
List<Backend> decommissionBackends = Lists.newArrayList();
// check if exist
- for (Pair<String, Integer> pair : hostPortPairs) {
- Backend backend = infoService.getBackendWithHeartbeatPort(pair.first, pair.second);
+ for (Triple<String, String, Integer> triple : ipHostPortTriples) {
+ Backend backend = infoService.getBackendWithHeartbeatPort(triple.getLeft(), triple.getMiddle(), triple.getRight());
if (backend == null) {
- throw new DdlException("Backend does not exist[" + pair.first + ":" + pair.second + "]");
+ throw new DdlException("Backend does not exist[" + triple.getLeft() + ":" + triple.getLeft() + "]");
Review Comment:
my fault, I will fix it
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] stalary commented on a diff in pull request #9172: [enhancement](k8s) Support k8s_container_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
stalary commented on code in PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#discussion_r858767954
##########
fe/fe-core/src/main/java/org/apache/doris/system/SystemInfoService.java:
##########
@@ -1061,7 +1084,7 @@ public static Pair<String, Integer> validateHostAndPort(String hostPort) throws
throw new AnalysisException("Port is out of range: " + heartbeatPort);
}
- return new Pair<String, Integer>(host, heartbeatPort);
+ return Triple.of(ip, Config.enable_k8s_container_drift_mode ? hostName : null, heartbeatPort);
Review Comment:
Ok, I see your point. Thanks.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yiguolei commented on a diff in pull request #9172: [enhancement](k8s) Support k8s_container_detect_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#discussion_r861577375
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/BrokerMgr.java:
##########
@@ -306,6 +306,22 @@ public void replayDropAllBroker(String name) {
}
}
+ public void replayUpdateBrokerInfo(String name, List<FsBroker> brokers) {
+ lock.lock();
+ try {
+ for (FsBroker oldBroker : brokerListMap.get(name)) {
+ for (FsBroker newBroker : brokers) {
+ if (oldBroker.hostName != null && oldBroker.hostName.equals(newBroker.hostName)
+ && oldBroker.port == newBroker.port) {
+ oldBroker.ip = newBroker.ip;
Review Comment:
something is wrong here. brokermap has ip-->broker. If changed ip here, should also change the map.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] caiconghui commented on a diff in pull request #9172: [enhancement](k8s) Support fqdn mode for be in k8s enviroment
Posted by GitBox <gi...@apache.org>.
caiconghui commented on code in PR #9172:
URL: https://github.com/apache/doris/pull/9172#discussion_r1032902177
##########
fe/fe-core/src/main/java/org/apache/doris/system/FQDNManager.java:
##########
@@ -0,0 +1,73 @@
+// 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.doris.system;
+
+import org.apache.doris.catalog.Env;
+import org.apache.doris.common.ClientPool;
+import org.apache.doris.common.FeConstants;
+import org.apache.doris.common.util.MasterDaemon;
+import org.apache.doris.thrift.TNetworkAddress;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+public class FQDNManager extends MasterDaemon {
+ private static final Logger LOG = LogManager.getLogger(FQDNManager.class);
+
+ private SystemInfoService nodeMgr;
+
+ public FQDNManager(SystemInfoService nodeMgr) {
+ super("FQDN mgr", FeConstants.ip_check_interval_second * 1000L);
+ this.nodeMgr = nodeMgr;
+ }
+
+ /**
+ * At each round: check if ip of be has already been changed
+ */
+ @Override
+ protected void runAfterCatalogReady() {
+ for (Backend be : nodeMgr.getIdToBackend().values()) {
+ if (be.getHostName() != null) {
+ try {
+ InetAddress inetAddress = InetAddress.getByName(be.getHostName());
+ if (!be.getHost().equalsIgnoreCase(inetAddress.getHostAddress())) {
+ String ip = be.getHost();
+ if (!ip.equalsIgnoreCase("unknown")) {
+ ClientPool.backendPool.clearPool(new TNetworkAddress(ip, be.getBePort()));
+ }
+ be.setHost(inetAddress.getHostAddress());
+ Env.getCurrentEnv().getEditLog().logBackendStateChange(be);
+ LOG.warn("ip for {} of be has been changed from {} to {}", be.getHostName(), ip, be.getHost());
+ }
+ } catch (UnknownHostException e) {
+ LOG.warn("unknown host name for be, {}", be.getHostName(), e);
+ if (!be.isAlive() && !be.getHost().equalsIgnoreCase("unknown")) {
+ String ip = be.getHost();
+ ClientPool.backendPool.clearPool(new TNetworkAddress(ip, be.getBePort()));
+ be.setHost("unknown");
Review Comment:
ok
##########
fe/fe-core/src/main/java/org/apache/doris/system/FQDNManager.java:
##########
@@ -0,0 +1,73 @@
+// 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.doris.system;
+
+import org.apache.doris.catalog.Env;
+import org.apache.doris.common.ClientPool;
+import org.apache.doris.common.FeConstants;
+import org.apache.doris.common.util.MasterDaemon;
+import org.apache.doris.thrift.TNetworkAddress;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+public class FQDNManager extends MasterDaemon {
+ private static final Logger LOG = LogManager.getLogger(FQDNManager.class);
+
+ private SystemInfoService nodeMgr;
+
+ public FQDNManager(SystemInfoService nodeMgr) {
+ super("FQDN mgr", FeConstants.ip_check_interval_second * 1000L);
+ this.nodeMgr = nodeMgr;
+ }
+
+ /**
+ * At each round: check if ip of be has already been changed
+ */
+ @Override
+ protected void runAfterCatalogReady() {
+ for (Backend be : nodeMgr.getIdToBackend().values()) {
+ if (be.getHostName() != null) {
+ try {
+ InetAddress inetAddress = InetAddress.getByName(be.getHostName());
+ if (!be.getHost().equalsIgnoreCase(inetAddress.getHostAddress())) {
+ String ip = be.getHost();
+ if (!ip.equalsIgnoreCase("unknown")) {
+ ClientPool.backendPool.clearPool(new TNetworkAddress(ip, be.getBePort()));
+ }
+ be.setHost(inetAddress.getHostAddress());
+ Env.getCurrentEnv().getEditLog().logBackendStateChange(be);
+ LOG.warn("ip for {} of be has been changed from {} to {}", be.getHostName(), ip, be.getHost());
+ }
+ } catch (UnknownHostException e) {
+ LOG.warn("unknown host name for be, {}", be.getHostName(), e);
+ if (!be.isAlive() && !be.getHost().equalsIgnoreCase("unknown")) {
Review Comment:
here is to make ip work if dns has some problem when be is still alive.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on pull request #9172: [enhancement](k8s) Support k8s_container_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
caiconghui commented on PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#issuecomment-1109829258
> `triple.getMiddle() == null ? triple.getLeft() : triple.getMiddle()` This code appears many times, can we do it in the generated place? `return Triple.of(ip, Config.enable_k8s_container_drift_mode ? hostName : ip, heartbeatPort)` whether it can be satisfied?
Config.enable_k8s_container_drift_mode config is master only, replay operation no need depend on this config.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yiguolei commented on a diff in pull request #9172: [enhancement](k8s) Support k8s_container_detect_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#discussion_r861518506
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/BackendClause.java:
##########
@@ -32,26 +33,27 @@
public class BackendClause extends AlterClause {
protected List<String> hostPorts;
- protected List<Pair<String, Integer>> hostPortPairs;
+ protected List<Triple<String, String, Integer>> ipHostPorTriples;
protected BackendClause(List<String> hostPorts) {
super(AlterOpType.ALTER_OTHER);
this.hostPorts = hostPorts;
- this.hostPortPairs = new LinkedList<Pair<String, Integer>>();
+ this.ipHostPorTriples = new LinkedList<>();
}
- public List<Pair<String, Integer>> getHostPortPairs() {
- return hostPortPairs;
+ public List<Triple<String, String, Integer>> getIpHostPorTriples() {
Review Comment:
getIpHostPortTriples ?
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yiguolei commented on a diff in pull request #9172: [enhancement](k8s) Support k8s_container_detect_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#discussion_r861575405
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/BrokerMgr.java:
##########
@@ -195,19 +195,19 @@ public void addBrokers(String name, Collection<Pair<String, Integer>> addresses)
}
List<FsBroker> addedBrokerAddress = Lists.newArrayList();
- for (Pair<String, Integer> pair : addresses) {
- List<FsBroker> addressList = brokerAddrsMap.get(pair.first);
+ for (Triple<String, String, Integer> triple : addresses) {
+ List<FsBroker> addressList = brokerAddrsMap.get(triple.getLeft());
for (FsBroker addr : addressList) {
- if (addr.port == pair.second) {
- throw new DdlException("Broker(" + pair.first + ":" + pair.second
+ if (addr.port == triple.getRight()) {
Review Comment:
if drift mode is set, should also check hostname here
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] caiconghui merged pull request #9172: [enhancement](k8s) Support fqdn mode for be in k8s enviroment
Posted by GitBox <gi...@apache.org>.
caiconghui merged PR #9172:
URL: https://github.com/apache/doris/pull/9172
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a diff in pull request #9172: [enhancement](k8s) Support k8s_container_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
caiconghui commented on code in PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#discussion_r858765531
##########
fe/fe-core/src/main/java/org/apache/doris/system/SystemInfoService.java:
##########
@@ -1061,7 +1084,7 @@ public static Pair<String, Integer> validateHostAndPort(String hostPort) throws
throw new AnalysisException("Port is out of range: " + heartbeatPort);
}
- return new Pair<String, Integer>(host, heartbeatPort);
+ return Triple.of(ip, Config.enable_k8s_container_drift_mode ? hostName : null, heartbeatPort);
Review Comment:
for broker, now we use ip as unique identifier, but in k8s environment, we need to use hostname, so here I make hostName null if doris not in k8s environment, or we will need to do more check for hostName is really hostName, or it is a ip? may be more complex. so if we don't need it, just set it null
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] stalary commented on pull request #9172: [enhancement](k8s) Support k8s_container_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
stalary commented on PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#issuecomment-1110471307
LGTM
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] caiconghui commented on a diff in pull request #9172: [enhancement](k8s) Support fqdn mode for be in k8s enviroment
Posted by GitBox <gi...@apache.org>.
caiconghui commented on code in PR #9172:
URL: https://github.com/apache/doris/pull/9172#discussion_r1032901887
##########
fe/fe-core/src/main/java/org/apache/doris/system/FQDNManager.java:
##########
@@ -0,0 +1,73 @@
+// 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.doris.system;
+
+import org.apache.doris.catalog.Env;
+import org.apache.doris.common.ClientPool;
+import org.apache.doris.common.FeConstants;
+import org.apache.doris.common.util.MasterDaemon;
+import org.apache.doris.thrift.TNetworkAddress;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+public class FQDNManager extends MasterDaemon {
+ private static final Logger LOG = LogManager.getLogger(FQDNManager.class);
+
+ private SystemInfoService nodeMgr;
+
+ public FQDNManager(SystemInfoService nodeMgr) {
+ super("FQDN mgr", FeConstants.ip_check_interval_second * 1000L);
+ this.nodeMgr = nodeMgr;
+ }
+
+ /**
+ * At each round: check if ip of be has already been changed
+ */
+ @Override
+ protected void runAfterCatalogReady() {
+ for (Backend be : nodeMgr.getIdToBackend().values()) {
+ if (be.getHostName() != null) {
+ try {
+ InetAddress inetAddress = InetAddress.getByName(be.getHostName());
+ if (!be.getHost().equalsIgnoreCase(inetAddress.getHostAddress())) {
+ String ip = be.getHost();
+ if (!ip.equalsIgnoreCase("unknown")) {
+ ClientPool.backendPool.clearPool(new TNetworkAddress(ip, be.getBePort()));
Review Comment:
actually if ip of pod changed， it means the pod has been reconstructed already， so I think it is ok to clear the client pool before changing BE'ip and writing edit log.
And pod ip changed is driven by k8s, not by Manual operation，in other word，pod‘s ip is also stable，if the host machine not failed or there is not manual reconstructed pod operation
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] yiguolei commented on a diff in pull request #9172: [enhancement](k8s) Support fqdn mode for be in k8s enviroment
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9172:
URL: https://github.com/apache/doris/pull/9172#discussion_r1035559094
##########
fe/fe-core/src/main/java/org/apache/doris/system/SystemInfoService.java:
##########
@@ -901,7 +915,8 @@ public void clear() {
this.idToReportVersionRef = null;
}
- public static Pair<String, Integer> validateHostAndPort(String hostPort) throws AnalysisException {
+ public static Triple<String, String, Integer> getIpHostAndPort(String hostPort, boolean strictCheck)
Review Comment:
Is it better to add a class instead of using pair or triple....
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #9172: [enhancement](k8s) Support fqdn mode for be in k8s enviroment
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9172:
URL: https://github.com/apache/doris/pull/9172#issuecomment-1331799234
PR approved by at least one committer and no changes requested.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #9172: [enhancement](k8s) Support fqdn mode for be in k8s enviroment
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9172:
URL: https://github.com/apache/doris/pull/9172#issuecomment-1331799290
PR approved by anyone and no changes requested.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] yiguolei commented on a diff in pull request #9172: [enhancement](k8s) Support fqdn mode for be in k8s enviroment
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9172:
URL: https://github.com/apache/doris/pull/9172#discussion_r1035566659
##########
fe/fe-core/src/main/java/org/apache/doris/system/Backend.java:
##########
@@ -70,7 +69,9 @@ public enum BackendState {
@SerializedName("id")
private long id;
@SerializedName("host")
- private String host;
+ private volatile String host;
Review Comment:
我们最好在这里做一下区分，ipaddress 和 host name， 感觉现在这个host 和 hostname 区分不清楚。
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yiguolei commented on a diff in pull request #9172: [enhancement](k8s) Support k8s_container_detect_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#discussion_r861552190
##########
fe/fe-core/src/main/java/org/apache/doris/system/FQDNManager.java:
##########
@@ -0,0 +1,103 @@
+// 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.doris.system;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import org.apache.doris.catalog.BrokerMgr;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.FsBroker;
+import org.apache.doris.common.FeConstants;
+import org.apache.doris.common.util.MasterDaemon;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.List;
+import java.util.Map;
+
+public class FQDNManager extends MasterDaemon {
+ private static final Logger LOG = LogManager.getLogger(FQDNManager.class);
+
+ private SystemInfoService nodeMgr;
+
+ public FQDNManager(SystemInfoService nodeMgr) {
+ super("FQDN mgr", FeConstants.ip_check_interval_second * 1000L);
+ this.nodeMgr = nodeMgr;
+ }
+
+ /**
+ * At each round: check if ip of be and broker has already been changed
+ */
+ @Override
+ protected void runAfterCatalogReady() {
+ for (Backend be : nodeMgr.getIdToBackend().values()) {
+ if (be.getHostName() != null) {
+ try {
+ InetAddress inetAddress = InetAddress.getByName(be.getHostName());
+ if (!be.getHost().equalsIgnoreCase(inetAddress.getHostAddress())) {
+ String ip = be.getHost();
+ be.setHost(inetAddress.getHostAddress());
+ Catalog.getCurrentCatalog().getEditLog().logBackendStateChange(be);
+ LOG.warn("ip for {} of be has been changed from {} to {}", be.getHostName(), ip, be.getHost());
+ }
+ } catch (UnknownHostException e) {
+ LOG.warn("unknown host name for be, {}", be.getHostName(), e);
+ if (!be.isAlive()) {
+ String ip = be.getHost();
+ be.setHost("unknown");
+ Catalog.getCurrentCatalog().getEditLog().logBackendStateChange(be);
+ LOG.warn("ip for {} of be has been changed from {} to {}", be.getHostName(), ip, "unknown");
+ }
+ }
+ }
+ }
+
+ Map<String, List<FsBroker>> brokerMap = Maps.newHashMap(
+ Catalog.getCurrentCatalog().getBrokerMgr().getBrokerListMap());
+ for (Map.Entry<String, List<FsBroker>> entry : brokerMap.entrySet()) {
+ for (FsBroker broker : entry.getValue()) {
+ if (broker.hostName != null) {
+ try {
+ InetAddress inetAddress = InetAddress.getByName(broker.hostName);
+ if (!broker.ip.equalsIgnoreCase(inetAddress.getHostAddress())) {
Review Comment:
And here.
##########
fe/fe-core/src/main/java/org/apache/doris/system/FQDNManager.java:
##########
@@ -0,0 +1,103 @@
+// 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.doris.system;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import org.apache.doris.catalog.BrokerMgr;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.FsBroker;
+import org.apache.doris.common.FeConstants;
+import org.apache.doris.common.util.MasterDaemon;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.List;
+import java.util.Map;
+
+public class FQDNManager extends MasterDaemon {
+ private static final Logger LOG = LogManager.getLogger(FQDNManager.class);
+
+ private SystemInfoService nodeMgr;
+
+ public FQDNManager(SystemInfoService nodeMgr) {
+ super("FQDN mgr", FeConstants.ip_check_interval_second * 1000L);
+ this.nodeMgr = nodeMgr;
+ }
+
+ /**
+ * At each round: check if ip of be and broker has already been changed
+ */
+ @Override
+ protected void runAfterCatalogReady() {
+ for (Backend be : nodeMgr.getIdToBackend().values()) {
+ if (be.getHostName() != null) {
+ try {
+ InetAddress inetAddress = InetAddress.getByName(be.getHostName());
+ if (!be.getHost().equalsIgnoreCase(inetAddress.getHostAddress())) {
+ String ip = be.getHost();
+ be.setHost(inetAddress.getHostAddress());
+ Catalog.getCurrentCatalog().getEditLog().logBackendStateChange(be);
+ LOG.warn("ip for {} of be has been changed from {} to {}", be.getHostName(), ip, be.getHost());
+ }
+ } catch (UnknownHostException e) {
+ LOG.warn("unknown host name for be, {}", be.getHostName(), e);
+ if (!be.isAlive()) {
+ String ip = be.getHost();
+ be.setHost("unknown");
+ Catalog.getCurrentCatalog().getEditLog().logBackendStateChange(be);
+ LOG.warn("ip for {} of be has been changed from {} to {}", be.getHostName(), ip, "unknown");
+ }
+ }
+ }
+ }
+
+ Map<String, List<FsBroker>> brokerMap = Maps.newHashMap(
+ Catalog.getCurrentCatalog().getBrokerMgr().getBrokerListMap());
+ for (Map.Entry<String, List<FsBroker>> entry : brokerMap.entrySet()) {
+ for (FsBroker broker : entry.getValue()) {
+ if (broker.hostName != null) {
Review Comment:
using getter not access property directly.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] yiguolei commented on a diff in pull request #9172: [enhancement](k8s) Support k8s_container_detect_drift_mode for be and broker in k8s enviroment
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #9172:
URL: https://github.com/apache/incubator-doris/pull/9172#discussion_r861552013
##########
fe/fe-core/src/main/java/org/apache/doris/system/FQDNManager.java:
##########
@@ -0,0 +1,103 @@
+// 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.doris.system;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import org.apache.doris.catalog.BrokerMgr;
+import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.FsBroker;
+import org.apache.doris.common.FeConstants;
+import org.apache.doris.common.util.MasterDaemon;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.List;
+import java.util.Map;
+
+public class FQDNManager extends MasterDaemon {
+ private static final Logger LOG = LogManager.getLogger(FQDNManager.class);
+
+ private SystemInfoService nodeMgr;
+
+ public FQDNManager(SystemInfoService nodeMgr) {
+ super("FQDN mgr", FeConstants.ip_check_interval_second * 1000L);
+ this.nodeMgr = nodeMgr;
+ }
+
+ /**
+ * At each round: check if ip of be and broker has already been changed
+ */
+ @Override
+ protected void runAfterCatalogReady() {
+ for (Backend be : nodeMgr.getIdToBackend().values()) {
+ if (be.getHostName() != null) {
+ try {
+ InetAddress inetAddress = InetAddress.getByName(be.getHostName());
+ if (!be.getHost().equalsIgnoreCase(inetAddress.getHostAddress())) {
+ String ip = be.getHost();
+ be.setHost(inetAddress.getHostAddress());
+ Catalog.getCurrentCatalog().getEditLog().logBackendStateChange(be);
+ LOG.warn("ip for {} of be has been changed from {} to {}", be.getHostName(), ip, be.getHost());
+ }
+ } catch (UnknownHostException e) {
+ LOG.warn("unknown host name for be, {}", be.getHostName(), e);
+ if (!be.isAlive()) {
+ String ip = be.getHost();
+ be.setHost("unknown");
+ Catalog.getCurrentCatalog().getEditLog().logBackendStateChange(be);
+ LOG.warn("ip for {} of be has been changed from {} to {}", be.getHostName(), ip, "unknown");
+ }
+ }
+ }
+ }
+
+ Map<String, List<FsBroker>> brokerMap = Maps.newHashMap(
+ Catalog.getCurrentCatalog().getBrokerMgr().getBrokerListMap());
+ for (Map.Entry<String, List<FsBroker>> entry : brokerMap.entrySet()) {
+ for (FsBroker broker : entry.getValue()) {
+ if (broker.hostName != null) {
Review Comment:
using getter not access property directly.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] caiconghui commented on pull request #9172: [enhancement](k8s) Support fqdn mode for be in k8s enviroment
Posted by GitBox <gi...@apache.org>.
caiconghui commented on PR #9172:
URL: https://github.com/apache/doris/pull/9172#issuecomment-1328157457
1. enable_fqdn_mode is mainly used in k8s environment, not be compatible with old cluster, when set true, the FQDN may not work, because origin's be hostName is null, but would not affect the behavior of the whole cluster.
2. when enable_fqdn_mode is true, the FDQNManager would detect be ip change every some seconds, and would check hostName needed by backend when add backend, you sholud specify "hostName:port" or still give "ip:port" with that cluster can get hostName by ip, otherwise, add add backend would failed.
3. now for be, if drop or modify backend cluster would check backend's hostname and port is same or then ip and port is same.
4. check interval is 5 seconds, it means FDQNManager would check every 5 seconds, and other value is also ok, it just affect the be ip detect time
5. if be's ip change, then the FDQNManager would invalidate all client cache to be, and set the new ip for the be.
6. finally all be and fe connection is still based all ip, this logic keep unchanged
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] caiconghui commented on a diff in pull request #9172: [enhancement](k8s) Support fqdn mode for be in k8s enviroment
Posted by GitBox <gi...@apache.org>.
caiconghui commented on code in PR #9172:
URL: https://github.com/apache/doris/pull/9172#discussion_r1032899046
##########
fe/fe-core/src/main/java/org/apache/doris/system/SystemInfoService.java:
##########
@@ -901,7 +915,8 @@ public void clear() {
this.idToReportVersionRef = null;
}
- public static Pair<String, Integer> validateHostAndPort(String hostPort) throws AnalysisException {
+ public static Triple<String, String, Integer> getIpHostAndPort(String hostPort, boolean strictCheck)
Review Comment:
because when pod is deconstructed，the ip of pod may not be obtained，to prevent that we cannot drop backend
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org