Skip to content

Commit f5d4e89

Browse files
committed
fix: Fix SSH connection error with locking
1 parent eadc54d commit f5d4e89

File tree

1 file changed

+66
-36
lines changed

1 file changed

+66
-36
lines changed

java/mc-o11y-manager/src/main/java/com/mcmp/o11ymanager/manager/service/SshServiceImpl.java

Lines changed: 66 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ public class SshServiceImpl implements SshService {
5959
public static final int SSH_COMMAND_EXECUTE_FAILED_CODE = -999;
6060

6161
private final Map<String, SshConnection> connectionCache = new ConcurrentHashMap<>();
62+
private final Map<String, Object> connectionLocks = new ConcurrentHashMap<>();
63+
64+
private Object getConnectionLock(String cacheKey) {
65+
return connectionLocks.computeIfAbsent(cacheKey, k -> new Object());
66+
}
6267

6368
private String getTelegrafServiceName() {
6469
return "cmp-telegraf-" + deploySiteCode + ".service";
@@ -104,33 +109,45 @@ public SshConnection getConnectionWithPrivateKey(
104109
}
105110

106111
String cacheKey = ip + ":" + port + ":" + user;
112+
113+
// Check cache for active connection first (without lock)
107114
SshConnection connection = connectionCache.get(cacheKey);
115+
if (connection != null && connection.isActive()) {
116+
connection.updateLastUsedTime();
117+
return connection;
118+
}
108119

109-
if (connection != null) {
110-
if (connection.isActive()) {
120+
// Synchronize with host-based lock to prevent concurrent connection attempts
121+
synchronized (getConnectionLock(cacheKey)) {
122+
// Double-check (another thread may have already connected)
123+
connection = connectionCache.get(cacheKey);
124+
if (connection != null && connection.isActive()) {
125+
connection.updateLastUsedTime();
111126
return connection;
112-
} else {
127+
}
128+
129+
// Clean up inactive connection
130+
if (connection != null) {
113131
try {
114132
connection.close();
115133
} catch (IOException e) {
116-
log.error("Error closing stale connection: {}", e.getMessage());
117-
throw new SshConnectionException(requestInfo.getRequestId(), ip);
134+
log.debug("Error closing stale connection: {}", e.getMessage());
118135
}
136+
connectionCache.remove(cacheKey);
119137
}
120-
}
121138

122-
try {
123-
connection = sshPort.openSessionWithPrivateKey(user, ip, port, privateKeyPath);
124-
connectionCache.put(cacheKey, connection);
125-
connection.updateLastUsedTime();
126-
127-
return connection;
128-
} catch (Exception e) {
129-
log.error(
130-
"Error establishing SSH connection with private key to {}: {}",
131-
ip,
132-
e.getMessage());
133-
throw new SshConnectionException(requestInfo.getRequestId(), ip);
139+
try {
140+
connection = sshPort.openSessionWithPrivateKey(user, ip, port, privateKeyPath);
141+
connectionCache.put(cacheKey, connection);
142+
connection.updateLastUsedTime();
143+
return connection;
144+
} catch (Exception e) {
145+
log.error(
146+
"Error establishing SSH connection with private key to {}: {}",
147+
ip,
148+
e.getMessage());
149+
throw new SshConnectionException(requestInfo.getRequestId(), ip);
150+
}
134151
}
135152
}
136153

@@ -146,33 +163,46 @@ public SshConnection getConnectionWithPrivateKeyString(
146163
}
147164

148165
String cacheKey = ip + ":" + port + ":" + user;
166+
167+
// Check cache for active connection first (without lock)
149168
SshConnection connection = connectionCache.get(cacheKey);
169+
if (connection != null && connection.isActive()) {
170+
connection.updateLastUsedTime();
171+
return connection;
172+
}
150173

151-
if (connection != null) {
152-
if (connection.isActive()) {
174+
// Synchronize with host-based lock to prevent concurrent connection attempts
175+
synchronized (getConnectionLock(cacheKey)) {
176+
// Double-check (another thread may have already connected)
177+
connection = connectionCache.get(cacheKey);
178+
if (connection != null && connection.isActive()) {
179+
connection.updateLastUsedTime();
153180
return connection;
154-
} else {
181+
}
182+
183+
// Clean up inactive connection
184+
if (connection != null) {
155185
try {
156186
connection.close();
157187
} catch (IOException e) {
158-
log.error("Error closing stale connection: {}", e.getMessage());
159-
throw new SshConnectionException(requestInfo.getRequestId(), ip);
188+
log.debug("Error closing stale connection: {}", e.getMessage());
160189
}
190+
connectionCache.remove(cacheKey);
161191
}
162-
}
163-
164-
try {
165-
connection = sshPort.openSessionWithPrivateKeyString(user, ip, port, privateKeyContent);
166-
connectionCache.put(cacheKey, connection);
167-
connection.updateLastUsedTime();
168192

169-
return connection;
170-
} catch (Exception e) {
171-
log.debug(
172-
"Error establishing SSH connection with private key string to {}: {}",
173-
ip,
174-
e.getMessage());
175-
throw new SshConnectionException(requestInfo.getRequestId(), ip);
193+
try {
194+
connection =
195+
sshPort.openSessionWithPrivateKeyString(user, ip, port, privateKeyContent);
196+
connectionCache.put(cacheKey, connection);
197+
connection.updateLastUsedTime();
198+
return connection;
199+
} catch (Exception e) {
200+
log.debug(
201+
"Error establishing SSH connection with private key string to {}: {}",
202+
ip,
203+
e.getMessage());
204+
throw new SshConnectionException(requestInfo.getRequestId(), ip);
205+
}
176206
}
177207
}
178208

0 commit comments

Comments
 (0)