Skip to content

Commit c467686

Browse files
nvazqueznvazquez
authored
[ConsoleProxy] Performance fixes on concurrency and closing console sessions sockets and introduce reconnection window for console sessions (#13683)
* Add lock and synchronize allowed sessions * Fix closing connections to VNC ports on console close * Add reconnection grant window to prevent network connectivity issues after acquiring a session * Introduce zone setting to control the reconnection window for console sessions
1 parent 10d3e11 commit c467686

10 files changed

Lines changed: 159 additions & 15 deletions

File tree

‎server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManager.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ public interface ConsoleProxyManager extends Manager, ConsoleProxyService {
8484
ConfigKey<Boolean> ConsoleProxyDisableRpFilter = new ConfigKey<>(Boolean.class, "consoleproxy.disable.rpfilter", "Console Proxy", "true",
8585
"disable rp_filter on console proxy VM public interface", true, ConfigKey.Scope.Zone, null);
8686

87+
ConfigKey<Long> ConsoleProxySessionReconnectionWindow = new ConfigKey<>(Long.class, "consoleproxy.session.reconnection.window", "Console Proxy", "0",
88+
"Reconnection window (in milliseconds) for client IPs to the same session on console proxy VM", true, ConfigKey.Scope.Zone, null);
89+
8790
ConfigKey<Integer> ConsoleProxyLaunchMax = new ConfigKey<>(Integer.class, "consoleproxy.launch.max", "Console Proxy", "10",
8891
"maximum number of console proxy instances per zone can be launched", false, ConfigKey.Scope.Zone, null);
8992

‎server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1210,6 +1210,10 @@ public boolean finalizeVirtualMachineProfile(VirtualMachineProfile profile, Depl
12101210
if (Boolean.TRUE.equals(disableRpFilter)) {
12111211
buf.append(" disable_rp_filter=true");
12121212
}
1213+
Long sessionReconnectionWindow = ConsoleProxySessionReconnectionWindow.valueIn(datacenterId);
1214+
if (sessionReconnectionWindow != null && sessionReconnectionWindow > 0) {
1215+
buf.append(" session_reconnection_window=").append(sessionReconnectionWindow);
1216+
}
12131217

12141218
String msPublicKey = configurationDao.getValue("ssh.publickey");
12151219
buf.append(" authorized_key=").append(VirtualMachineGuru.getEncodedMsPublicKey(msPublicKey));
@@ -1588,7 +1592,7 @@ public ConfigKey<?>[] getConfigKeys() {
15881592
return new ConfigKey<?>[] {ConsoleProxySslEnabled, NoVncConsoleDefault, NoVncConsoleSourceIpCheckEnabled, ConsoleProxyServiceOffering,
15891593
ConsoleProxyCapacityStandby, ConsoleProxyCapacityScanInterval, ConsoleProxyRestart, ConsoleProxyUrlDomain, ConsoleProxySessionMax, ConsoleProxySessionTimeout, ConsoleProxyDisableRpFilter, ConsoleProxyLaunchMax,
15901594
ConsoleProxyManagementLastState, ConsoleProxyServiceManagementState, NoVncConsoleShowDot,
1591-
ConsoleProxyVmUserData};
1595+
ConsoleProxyVmUserData, ConsoleProxySessionReconnectionWindow};
15921596
}
15931597

15941598
protected ConsoleProxyStatus parseJsonToConsoleProxyStatus(String json) throws JsonParseException {

‎services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java‎

Lines changed: 80 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import java.net.InetSocketAddress;
2727
import java.net.URISyntaxException;
2828
import java.net.URL;
29-
import java.util.HashSet;
3029
import java.util.Hashtable;
3130
import java.util.Map;
3231
import java.util.Properties;
@@ -83,12 +82,73 @@ public class ConsoleProxy {
8382
static String encryptorPassword = "Dummy";
8483
static final String[] skipProperties = new String[]{"certificate", "cacertificate", "keystore_password", "privatekey"};
8584

86-
static Set<String> allowedSessions = new HashSet<>();
85+
static Set<String> allowedSessions = ConcurrentHashMap.newKeySet();
86+
private static final Object allowedSessionsLock = new Object();
8787

88+
private static final Map<String, ReconnectGrant> sessionReconnectGrants = new ConcurrentHashMap<>();
89+
private static long sessionReconnectionWindowMs = 0L;
90+
91+
// Invoked through reflection
8892
public static void addAllowedSession(String sessionUuid) {
8993
allowedSessions.add(sessionUuid);
9094
}
9195

96+
/**
97+
* Grant the client IP a reconnection window of #{@link #sessionReconnectionWindowMs} ms to the same session UUID in case of a disconnection.
98+
* The grant is bound to the client IP that was using the session so it cannot be redeemed by another client.
99+
* @param sessionUuid session UUID to grant a reconnect window for
100+
* @param clientIp source IP of the client the session was granted to
101+
*/
102+
public static void grantReconnectWindowForSessionAndClientIp(String sessionUuid, String clientIp) {
103+
if (sessionReconnectionWindowMs > 0) {
104+
ReconnectGrant grant = new ReconnectGrant(System.currentTimeMillis() + sessionReconnectionWindowMs, clientIp);
105+
sessionReconnectGrants.put(sessionUuid, grant);
106+
}
107+
}
108+
109+
/**
110+
* True if the session UUID has been granted reconnection, within the reconnection window #{@link #sessionReconnectionWindowMs}.
111+
*/
112+
private static boolean isSessionReconnectionGrantedForClientIp(String sessionUuid, String clientIp) {
113+
ReconnectGrant grant = sessionReconnectGrants.remove(sessionUuid);
114+
if (grant == null) {
115+
return false;
116+
}
117+
if (grant.isExpired(System.currentTimeMillis())) {
118+
LOGGER.warn("Rejecting reconnection for session {} as the reconnect window: {}ms is already expired",
119+
sessionUuid, sessionReconnectionWindowMs);
120+
return false;
121+
}
122+
if (grant.clientIp != null && !grant.clientIp.equals(clientIp)) {
123+
LOGGER.warn("Rejecting reconnection for session {} as it was requested from IP {} " +
124+
"but the session was granted to IP {}", sessionUuid, clientIp, grant.clientIp);
125+
return false;
126+
}
127+
return true;
128+
}
129+
130+
/**
131+
* Drops expired, unclaimed reconnect grants so sessionReconnectGrants doesn't grow unbounded
132+
* when a client never reconnects after a disconnection. Invoked periodically by {@link ConsoleProxyGCThread}.
133+
*/
134+
static void cleanupExpiredReconnectGrants() {
135+
sessionReconnectGrants.entrySet().removeIf(entry -> entry.getValue().isExpired(System.currentTimeMillis()));
136+
}
137+
138+
private static final class ReconnectGrant {
139+
final long expiryMillis;
140+
final String clientIp;
141+
142+
ReconnectGrant(long expiryMillis, String clientIp) {
143+
this.expiryMillis = expiryMillis;
144+
this.clientIp = clientIp;
145+
}
146+
147+
boolean isExpired(long now) {
148+
return now >= expiryMillis;
149+
}
150+
}
151+
92152
private static void configLog4j() {
93153
final ClassLoader loader = Thread.currentThread().getContextClassLoader();
94154
URL configUrl = loader.getResource("/conf/log4j-cloud.xml");
@@ -166,6 +226,12 @@ private static void configProxy(Properties conf) {
166226
defaultBufferSize = Integer.parseInt(s);
167227
LOGGER.info("Setting defaultBufferSize=" + defaultBufferSize);
168228
}
229+
230+
s = conf.getProperty("session_reconnection_window");
231+
if (s != null) {
232+
sessionReconnectionWindowMs = Long.parseLong(s);
233+
LOGGER.info("Setting sessionReconnectionWindowMs=" + sessionReconnectionWindowMs);
234+
}
169235
}
170236

171237
public static ConsoleProxyServerFactory getHttpServerFactory() {
@@ -209,13 +275,17 @@ public static ConsoleProxyAuthenticationResult authenticateConsoleAccess(Console
209275
}
210276

211277
String sessionUuid = param.getSessionUuid();
212-
if (allowedSessions.contains(sessionUuid)) {
213-
LOGGER.debug("Acquiring the session " + sessionUuid + " not available for future use");
214-
allowedSessions.remove(sessionUuid);
215-
} else {
216-
LOGGER.info("Session " + sessionUuid + " has already been used, cannot connect");
217-
authResult.setSuccess(false);
218-
return authResult;
278+
synchronized (allowedSessionsLock) {
279+
if (allowedSessions.remove(sessionUuid)) {
280+
LOGGER.debug("Acquiring the session {} from client IP {}", sessionUuid, param.getClientIp());
281+
} else if (isSessionReconnectionGrantedForClientIp(sessionUuid, param.getClientIp())) {
282+
LOGGER.info("Reconnecting the session {} after a dropped connection", sessionUuid);
283+
return authResult;
284+
} else {
285+
LOGGER.info("Invalid or already used session {}, cannot connect", sessionUuid);
286+
authResult.setSuccess(false);
287+
return authResult;
288+
}
219289
}
220290

221291
String websocketUrl = param.getWebsocketUrl();
@@ -625,7 +695,7 @@ public static ConsoleProxyNoVncClient getNoVncViewer(ConsoleProxyClientParam par
625695
} catch (IOException e) {
626696
LOGGER.error("Exception while disconnect session of novnc viewer object: " + viewer, e);
627697
}
628-
removeViewer(viewer);
698+
viewer.closeClient();
629699
viewer = new ConsoleProxyNoVncClient(session);
630700
viewer.initClient(param);
631701
connectionMap.put(clientKey, viewer);

‎services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyGCThread.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ public void run() {
7575

7676
while (true) {
7777
cleanupLogging();
78+
ConsoleProxy.cleanupExpiredReconnectGrants();
7879
bReportLoad = false;
7980

8081
if (logger.isDebugEnabled()) {

‎services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -175,12 +175,20 @@ private boolean checkSessionSourceIp(final Session session, final String sourceI
175175

176176
@OnWebSocketClose
177177
public void onClose(Session session, int statusCode, String reason) throws IOException, InterruptedException {
178-
String sessionSourceIp = session.getRemoteAddress().getAddress().getHostAddress();
179-
logger.debug("Closing WebSocket session [source IP: {}, status code: {}].", sessionSourceIp, statusCode);
180178
if (viewer != null) {
181-
ConsoleProxy.removeViewer(viewer);
179+
viewer.closeClient();
180+
}
181+
String sessionSourceIp = getRemoteAddressSafely(session);
182+
logger.debug("WebSocket session [source IP: {}, status code: {}, reason: {}] closed successfully.", sessionSourceIp, statusCode, reason);
183+
}
184+
185+
private String getRemoteAddressSafely(Session session) {
186+
try {
187+
return session.getRemoteAddress().getAddress().getHostAddress();
188+
} catch (Exception e) {
189+
logger.debug("Failed to get remote address from WebSocket session", e);
190+
return "unknown";
182191
}
183-
logger.debug("WebSocket session [source IP: {}, status code: {}] closed successfully.", sessionSourceIp, statusCode);
184192
}
185193

186194
@OnWebSocketFrame

‎services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ public void run() {
177177
}
178178
}
179179
logger.info("Connection with client [{}] [IP: {}] is dead.", clientId, clientSourceIp);
180+
ConsoleProxy.grantReconnectWindowForSessionAndClientIp(sessionUuid, clientSourceIp);
180181
} catch (IOException e) {
181182
logger.error("Error on VNC client", e);
182183
}
@@ -374,6 +375,9 @@ public void closeClient() {
374375
this.connectionAlive = false;
375376
// Clear buffer reference to allow GC when client disconnects
376377
this.readBuffer = null;
378+
if (client != null) {
379+
client.close();
380+
}
377381
ConsoleProxy.removeViewer(this);
378382
}
379383

‎services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,28 @@ public void proxyMsgOverWebSocketConnection(ByteBuffer msg) {
129129
}
130130
}
131131

132+
public void close() {
133+
if (nioSocketConnection != null) {
134+
nioSocketConnection.close();
135+
}
136+
if (webSocketReverseProxy != null) {
137+
webSocketReverseProxy.close();
138+
}
139+
if (socket != null) {
140+
try {
141+
if (is != null) {
142+
is.close();
143+
}
144+
if (os != null) {
145+
os.close();
146+
}
147+
socket.close();
148+
} catch (IOException e) {
149+
logger.debug("Error closing socket: " + e.getMessage(), e);
150+
}
151+
}
152+
}
153+
132154
private void setTunnelSocketStreams() throws IOException {
133155
this.is = new DataInputStream(this.socket.getInputStream());
134156
this.os = new DataOutputStream(this.socket.getOutputStream());

‎services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/network/NioSocket.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,4 +120,28 @@ protected int writeToSocketChannel(ByteBuffer buf, int len) {
120120
return 0;
121121
}
122122
}
123+
124+
public void close() {
125+
try {
126+
if (socketChannel != null) {
127+
socketChannel.close();
128+
}
129+
} catch (IOException e) {
130+
logger.debug("Error closing socket channel: " + e.getMessage(), e);
131+
}
132+
try {
133+
if (readSelector != null) {
134+
readSelector.close();
135+
}
136+
} catch (IOException e) {
137+
logger.debug("Error closing read selector: " + e.getMessage(), e);
138+
}
139+
try {
140+
if (writeSelector != null) {
141+
writeSelector.close();
142+
}
143+
} catch (IOException e) {
144+
logger.debug("Error closing write selector: " + e.getMessage(), e);
145+
}
146+
}
123147
}

‎services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/network/NioSocketHandler.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,5 @@ public interface NioSocketHandler {
4141
void flushWriteBuffer();
4242
void startTLSConnection(NioSocketSSLEngineManager sslEngineManager);
4343
boolean isTLSConnection();
44+
void close();
4445
}

‎services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/network/NioSocketHandlerImpl.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,12 @@ public class NioSocketHandlerImpl implements NioSocketHandler {
2828
private NioSocketInputStream inputStream;
2929
private NioSocketOutputStream outputStream;
3030
private boolean isTLS = false;
31+
private final NioSocket socket;
3132

3233
protected Logger logger = LogManager.getLogger(getClass());
3334

3435
public NioSocketHandlerImpl(NioSocket socket) {
36+
this.socket = socket;
3537
this.inputStream = new NioSocketInputStream(ConsoleProxy.defaultBufferSize, socket);
3638
this.outputStream = new NioSocketOutputStream(ConsoleProxy.defaultBufferSize, socket);
3739
}
@@ -109,4 +111,9 @@ public NioSocketInputStream getInputStream() {
109111
public NioSocketOutputStream getOutputStream() {
110112
return outputStream;
111113
}
114+
115+
@Override
116+
public void close() {
117+
socket.close();
118+
}
112119
}

0 commit comments

Comments
 (0)