diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/clustermonitor/ClusterStatsJdbcMonitor.java b/gateway-ha/src/main/java/io/trino/gateway/ha/clustermonitor/ClusterStatsJdbcMonitor.java index 0df56f0e3..6175c1006 100644 --- a/gateway-ha/src/main/java/io/trino/gateway/ha/clustermonitor/ClusterStatsJdbcMonitor.java +++ b/gateway-ha/src/main/java/io/trino/gateway/ha/clustermonitor/ClusterStatsJdbcMonitor.java @@ -32,6 +32,8 @@ import java.util.concurrent.Executors; import java.util.concurrent.TimeoutException; +import com.google.common.collect.ImmutableMap; + import static java.util.concurrent.TimeUnit.SECONDS; public class ClusterStatsJdbcMonitor @@ -39,7 +41,8 @@ public class ClusterStatsJdbcMonitor { private static final Logger log = Logger.get(ClusterStatsJdbcMonitor.class); - private final Properties properties; // TODO Avoid using a mutable field + private final ImmutableMap properties; // Replaced mutable Properties with ImmutableMap + private final JdbcMonitorConfiguration jdbcConfig; private final Duration queryTimeout; private static final String STATE_QUERY = "SELECT state, COUNT(*) as count " @@ -49,16 +52,17 @@ public class ClusterStatsJdbcMonitor public ClusterStatsJdbcMonitor(BackendStateConfiguration backendStateConfiguration, MonitorConfiguration monitorConfiguration) { - properties = new Properties(); - properties.setProperty("user", backendStateConfiguration.getUsername()); - properties.setProperty("password", backendStateConfiguration.getPassword()); - properties.setProperty("SSL", String.valueOf(backendStateConfiguration.getSsl())); - // explicitPrepare is a valid property for Trino versions >= 431. To avoid compatibility - // issues with versions < 431, this property is left unset when explicitPrepare=true, which is the default - if (!monitorConfiguration.isExplicitPrepare()) { - properties.setProperty("explicitPrepare", "false"); - } + this.jdbcConfig = JdbcMonitorConfiguration.from(backendStateConfiguration, monitorConfiguration); queryTimeout = monitorConfiguration.getQueryTimeout(); + + // Create immutable properties map to avoid mutable field anti-pattern + Properties baseProperties = jdbcConfig.toProperties(); + ImmutableMap.Builder propertiesBuilder = ImmutableMap.builder(); + for (String key : baseProperties.stringPropertyNames()) { + propertiesBuilder.put(key, baseProperties.getProperty(key)); + } + this.properties = propertiesBuilder.build(); + log.info("state check configured"); } @@ -68,24 +72,28 @@ public ClusterStats monitor(ProxyBackendConfiguration backend) String url = backend.getProxyTo(); ClusterStats.Builder clusterStats = ClusterStatsMonitor.getClusterStatsBuilder(backend); String jdbcUrl; + Properties connectionProperties; try { URL parsedUrl = new URL(url); jdbcUrl = String .format("jdbc:trino://%s:%s/system", parsedUrl.getHost(), parsedUrl.getPort() == -1 ? parsedUrl.getDefaultPort() : parsedUrl.getPort()); - // automatically set ssl config based on url protocol - properties.setProperty("SSL", String.valueOf(parsedUrl.getProtocol().equals("https"))); + + // Create connection properties from immutable map + connectionProperties = new Properties(); + connectionProperties.putAll(properties); + connectionProperties.setProperty("SSL", String.valueOf(parsedUrl.getProtocol().equals("https"))); } catch (MalformedURLException e) { - log.error("could not parse backend url %s ", url); - return clusterStats.build(); // TODO Invalid configuration should fail + log.error("Invalid backend URL configuration: %s", url); + throw new IllegalArgumentException("Invalid backend URL: " + url, e); } - try (Connection conn = DriverManager.getConnection(jdbcUrl, properties); + try (Connection conn = DriverManager.getConnection(jdbcUrl, connectionProperties); PreparedStatement statement = SimpleTimeLimiter.create(Executors.newSingleThreadExecutor()).callWithTimeout( () -> conn.prepareStatement(STATE_QUERY), 10, SECONDS)) { - statement.setString(1, (String) properties.get("user")); + statement.setString(1, jdbcConfig.getUsername()); statement.setQueryTimeout((int) queryTimeout.roundTo(SECONDS)); Map partialState = new HashMap<>(); ResultSet rs = statement.executeQuery(); diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/resource/GatewayWebAppResource.java b/gateway-ha/src/main/java/io/trino/gateway/ha/resource/GatewayWebAppResource.java index 84723eea3..e20bc317e 100644 --- a/gateway-ha/src/main/java/io/trino/gateway/ha/resource/GatewayWebAppResource.java +++ b/gateway-ha/src/main/java/io/trino/gateway/ha/resource/GatewayWebAppResource.java @@ -94,8 +94,11 @@ public GatewayWebAppResource( this.queryHistoryManager = requireNonNull(queryHistoryManager, "queryHistoryManager is null"); this.backendStateManager = requireNonNull(backendStateManager, "backendStateManager is null"); this.resourceGroupsManager = requireNonNull(resourceGroupsManager, "resourceGroupsManager is null"); - this.uiConfiguration = configuration.getUiConfiguration(); this.routingRulesManager = requireNonNull(routingRulesManager, "routingRulesManager is null"); + + // UI configuration is immutable during application lifetime, no need to copy + this.uiConfiguration = configuration.getUiConfiguration(); + RoutingRulesConfiguration routingRules = configuration.getRoutingRules(); isRulesEngineEnabled = routingRules.isRulesEngineEnabled(); ruleType = routingRules.getRulesType();