Disable HTTP/2 for internal communication

There were reports of cluster instability after the HTTP/2 was enabled for internal communication. This requires more investigation and testing.
trinodb · Nov 28, 2024 · 59ac727 · 59ac727
1 parent dc64f02
commit 59ac727
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 7 deletions.
diff --git a/core/trino-main/src/main/java/io/trino/execution/TaskManagerConfig.java b/core/trino-main/src/main/java/io/trino/execution/TaskManagerConfig.java
@@ -74,7 +74,7 @@ public class TaskManagerConfig
     private Duration clientTimeout = new Duration(2, TimeUnit.MINUTES);
     private Duration infoMaxAge = new Duration(15, TimeUnit.MINUTES);
 
-    private Duration statusRefreshMaxWait = new Duration(3, TimeUnit.SECONDS);
+    private Duration statusRefreshMaxWait = new Duration(1, TimeUnit.SECONDS);
     private Duration infoUpdateInterval = new Duration(3, TimeUnit.SECONDS);
     private Duration taskTerminationTimeout = new Duration(1, TimeUnit.MINUTES);
 

diff --git a/core/trino-main/src/main/java/io/trino/server/InternalCommunicationConfig.java b/core/trino-main/src/main/java/io/trino/server/InternalCommunicationConfig.java
@@ -31,7 +31,7 @@
 public class InternalCommunicationConfig
 {
     private String sharedSecret;
-    private boolean http2Enabled = true;
+    private boolean http2Enabled;
     private boolean httpsRequired;
     private String keyStorePath;
     private String keyStorePassword;

diff --git a/core/trino-main/src/main/java/io/trino/server/ServerMainModule.java b/core/trino-main/src/main/java/io/trino/server/ServerMainModule.java
@@ -350,13 +350,21 @@ protected void setup(Binder binder)
 
         // exchange client
         binder.bind(DirectExchangeClientSupplier.class).to(DirectExchangeClientFactory.class).in(Scopes.SINGLETON);
+
+        InternalCommunicationConfig internalCommunicationConfig = buildConfigObject(InternalCommunicationConfig.class);
+
         install(internalHttpClientModule("exchange", ForExchange.class)
                 .withConfigDefaults(config -> {
                     config.setIdleTimeout(new Duration(30, SECONDS));
                     config.setRequestTimeout(new Duration(10, SECONDS));
-                    config.setMaxConnectionsPerServer(64);
                     config.setMaxContentLength(DataSize.of(32, MEGABYTE));
                     config.setMaxRequestsQueuedPerDestination(65536);
+                    if (internalCommunicationConfig.isHttp2Enabled()) {
+                        config.setMaxConnectionsPerServer(64);
+                    }
+                    else {
+                        config.setMaxConnectionsPerServer(250);
+                    }
                 }).build());
 
         configBinder(binder).bindConfig(DirectExchangeClientConfig.class);

diff --git a/core/trino-main/src/test/java/io/trino/execution/TestTaskManagerConfig.java b/core/trino-main/src/test/java/io/trino/execution/TestTaskManagerConfig.java
@@ -42,7 +42,7 @@ public void testDefaults()
                 .setThreadPerDriverSchedulerEnabled(true)
                 .setInitialSplitsPerNode(Runtime.getRuntime().availableProcessors() * 2)
                 .setSplitConcurrencyAdjustmentInterval(new Duration(100, TimeUnit.MILLISECONDS))
-                .setStatusRefreshMaxWait(new Duration(3, TimeUnit.SECONDS))
+                .setStatusRefreshMaxWait(new Duration(1, TimeUnit.SECONDS))
                 .setInfoUpdateInterval(new Duration(3, TimeUnit.SECONDS))
                 .setTaskTerminationTimeout(new Duration(1, TimeUnit.MINUTES))
                 .setPerOperatorCpuTimerEnabled(true)

diff --git a/core/trino-main/src/test/java/io/trino/server/TestInternalCommunicationConfig.java b/core/trino-main/src/test/java/io/trino/server/TestInternalCommunicationConfig.java
@@ -32,7 +32,7 @@ public void testDefaults()
     {
         assertRecordedDefaults(recordDefaults(InternalCommunicationConfig.class)
                 .setSharedSecret(null)
-                .setHttp2Enabled(true)
+                .setHttp2Enabled(false)
                 .setHttpsRequired(false)
                 .setKeyStorePath(null)
                 .setKeyStorePassword(null)
@@ -50,7 +50,7 @@ public void testExplicitPropertyMappings()
 
         Map<String, String> properties = ImmutableMap.<String, String>builder()
                 .put("internal-communication.shared-secret", "secret")
-                .put("internal-communication.http2.enabled", "false")
+                .put("internal-communication.http2.enabled", "true")
                 .put("internal-communication.https.required", "true")
                 .put("internal-communication.https.keystore.path", keystoreFile.toString())
                 .put("internal-communication.https.keystore.key", "key-key")
@@ -61,7 +61,7 @@ public void testExplicitPropertyMappings()
 
         InternalCommunicationConfig expected = new InternalCommunicationConfig()
                 .setSharedSecret("secret")
-                .setHttp2Enabled(false)
+                .setHttp2Enabled(true)
                 .setHttpsRequired(true)
                 .setKeyStorePath(keystoreFile.toString())
                 .setKeyStorePassword("key-key")