szetszwo commented on code in PR #11218:
URL: https://github.com/apache/ozone/pull/11218#discussion_r4096729810
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMFailoverProxyProviderBase.java:
##########
@@ -174,30 +174,120 @@ synchronized void replaceProxyInfoForTest(String nodeId,
SCMProxyInfo info) {
@VisibleForTesting
protected synchronized void loadConfigs() {
- List<SCMNodeInfo> scmNodeInfoList = SCMNodeInfo.buildNodeInfo(conf);
- scmNodeIds = new ArrayList<>();
+ ScmProxyConfig newConfig = buildConfigs();
+ scmNodeIds = newConfig.nodeIds;
+ scmProxyInfoMap.clear();
+ scmProxyInfoMap.putAll(newConfig.proxyInfoMap);
+ }
+ /**
+ * Resolve the node list and each node's address into a fresh, unshared
holder,
+ * touching no shared provider state so callers can run it without the
monitor.
+ * Throws when a node in the list has no address, leaving existing state
intact.
+ */
+ private ScmProxyConfig buildConfigs() {
+ List<SCMNodeInfo> scmNodeInfoList = SCMNodeInfo.buildNodeInfo(conf);
+ List<String> newScmNodeIds = new ArrayList<>();
+ Map<String, SCMProxyInfo> newScmProxyInfoMap = new HashMap<>();
for (SCMNodeInfo scmNodeInfo : scmNodeInfoList) {
String protocolAddress = getProtocolAddress(scmNodeInfo);
if (protocolAddress == null) {
throw new ConfigurationException(protocolClass.getSimpleName() + " SCM
Address could not " +
"be obtained from config. Config is not properly defined");
+ }
+ InetSocketAddress protocolAddr =
NetUtils.createSocketAddr(protocolAddress);
+
+ String scmServiceId = scmNodeInfo.getServiceId();
+ String scmNodeId = scmNodeInfo.getNodeId();
+ newScmNodeIds.add(scmNodeId);
+ // Preserve the original config string so DNS can be re-resolved on
+ // connection failure when the SCM peer is rescheduled to a new IP
+ // (Kubernetes pod-IP-change recovery). See refreshProxyAddressIfChanged.
+ SCMProxyInfo scmProxyInfo = new SCMProxyInfo(scmServiceId, scmNodeId,
+ protocolAddr, protocolAddress);
+ newScmProxyInfoMap.put(scmNodeId, scmProxyInfo);
+ }
+
+ return new ScmProxyConfig(newScmNodeIds, newScmProxyInfoMap);
+ }
Review Comment:
The `else` is unnecessarily removed here and the comment is reformatted.
Please revert them and minimize the change.
It took me 10 min to figure out such a simple change here.
```diff
+ private ScmProxyConfig buildConfigs() {
List<SCMNodeInfo> scmNodeInfoList = SCMNodeInfo.buildNodeInfo(conf);
- scmNodeIds = new ArrayList<>();
-
+ final List<String> newScmNodeIds = new ArrayList<>();
+ final Map<String, SCMProxyInfo> newScmProxyInfoMap = new HashMap<>();
for (SCMNodeInfo scmNodeInfo : scmNodeInfoList) {
String protocolAddress = getProtocolAddress(scmNodeInfo);
@@ -188,16 +199,96 @@ protected synchronized void loadConfigs() {
String scmServiceId = scmNodeInfo.getServiceId();
String scmNodeId = scmNodeInfo.getNodeId();
- scmNodeIds.add(scmNodeId);
+ newScmNodeIds.add(scmNodeId);
// Preserve the original config string so DNS can be re-resolved
// on connection failure when the SCM peer is rescheduled to a
// new IP (Kubernetes pod-IP-change recovery). See
// refreshProxyAddressIfChanged(String).
SCMProxyInfo scmProxyInfo = new SCMProxyInfo(scmServiceId,
scmNodeId,
protocolAddr, protocolAddress);
- scmProxyInfoMap.put(scmNodeId, scmProxyInfo);
+ newScmProxyInfoMap.put(scmNodeId, scmProxyInfo);
+ }
+ }
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
