hani-fouladgar commented on code in PR #11218:
URL: https://github.com/apache/ozone/pull/11218#discussion_r4097164971
##########
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:
Reverted to else as requested, though it's technically unnecessary since the
throw exits the method. Kept the `newScmProxyInfoMap`/`newScmNodeIds` locals
because `buildConfigs()` must not touch shared state — it returns a fresh
holder so `changeConfig()` can resolve addresses off-lock, and a mid-loop throw
leaves the live config untouched.
--
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]
