Fix dynamic table cpu burn #10472 (#10806)

danielkec · web-flow · commit ab5cbd26ff57 · 2025-11-07T17:05:05.000+01:00
* Fix dynamic table cpu burn #10472 Signed-off-by: Daniel Kec <daniel.kec@oracle.com> * H2Spec logging over jul Signed-off-by: Daniel Kec <daniel.kec@oracle.com> * Validate dyn table add after eviction is done Signed-off-by: Daniel Kec <daniel.kec@oracle.com> --------- Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
diff --git a/http/http2/src/main/java/io/helidon/http/http2/Http2Headers.java b/http/http2/src/main/java/io/helidon/http/http2/Http2Headers.java
@@ -1124,7 +1124,7 @@ void write(BufferData buffer) {
 
     /**
      * There is one dynamic table for inbound headers and one for outbound headers for each connection.
-     * This is to minimize size of headers on the transport.
+     * This is to minimize the size of headers on the transport.
      * The table caches header names and values and then uses indexes only when transferring headers over network.
      */
     public static class DynamicTable {
@@ -1175,8 +1175,8 @@ void maxTableSize(long number) {
             }
             this.maxTableSize = number;
             if (maxTableSize == 0) {
-                currentTableSize = 0;
                 headers.clear();
+                currentTableSize = 0;
             }
             while (maxTableSize < currentTableSize) {
                 evict();
@@ -1188,18 +1188,21 @@ int add(HeaderName headerName, String headerValue) {
             int size = name.length() + headerValue.getBytes(StandardCharsets.US_ASCII).length + 32;
 
             if (currentTableSize + size <= maxTableSize) {
+                // fast path
                 return add(headerName, headerValue, size);
             }
 
             while ((currentTableSize + size) > maxTableSize) {
                 evict();
-                if (currentTableSize <= 0) {
-                    throw new Http2Exception(Http2ErrorCode.COMPRESSION,
-                                             "Cannot add header record, max table size too low. "
-                                                     + "current size: " + currentTableSize + ", max size: " + maxTableSize + ","
-                                                     + " header size: " + size);
-                }
             }
+
+            if (maxTableSize - currentTableSize < size) {
+                throw new Http2Exception(Http2ErrorCode.COMPRESSION,
+                                         "Cannot add header record, max table size too low. "
+                                                 + "current size: " + currentTableSize + ", max size: " + maxTableSize + ","
+                                                 + " header size: " + size);
+            }
+
             return add(headerName, headerValue, size);
         }
 
@@ -1244,6 +1247,7 @@ private IndexedHeaderRecord find(HeaderName headerName, String headerValue) {
 
         private void evict() {
             if (headers.isEmpty()) {
+                currentTableSize = 0;
                 return;
             }
             DynamicHeader removed = headers.remove(headers.size() - 1);
diff --git a/http/http2/src/test/java/io/helidon/http/http2/DynamicTableTest.java b/http/http2/src/test/java/io/helidon/http/http2/DynamicTableTest.java
@@ -67,6 +67,46 @@ void testIssue10472() {
         table.maxTableSize(0L);
     }
 
+    @Test
+    void testIssue10472addOnlyOneFit() {
+        Http2Settings settings = Http2Settings.builder()
+                .add(Http2Setting.HEADER_TABLE_SIZE, 45L)
+                .build();
+
+        Http2Headers.DynamicTable table = Http2Headers.DynamicTable.create(settings);
+        table.add(HeaderNames.create("a"), "aa"); // 35
+        testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 1, "a", "aa");
+        table.add(HeaderNames.create("b"), "b"); // 34
+
+        testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 1, "b", "b");
+
+        table.add(HeaderNames.create("c"), "c"); // 34
+        table.add(HeaderNames.create("dddd"), "ddddd"); // 41
+
+        testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 1, "dddd", "ddddd");
+    }
+
+    @Test
+    void testIssue10472addOnlyTwoFit() {
+        Http2Settings settings = Http2Settings.builder()
+                .add(Http2Setting.HEADER_TABLE_SIZE, 75L)
+                .build();
+
+        Http2Headers.DynamicTable table = Http2Headers.DynamicTable.create(settings);
+        table.add(HeaderNames.create("a"), "aa"); // 35
+        testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 1, "a", "aa");
+        table.add(HeaderNames.create("b"), "b"); // 34
+
+        testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 1, "b", "b");
+        testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 2, "a", "aa");
+
+        table.add(HeaderNames.create("c"), "c"); // 34
+        table.add(HeaderNames.create("dddd"), "ddddd"); // 41
+
+        testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 1, "dddd", "ddddd");
+        testRecord(table, Http2Headers.StaticHeader.MAX_INDEX + 2, "c", "c");
+    }
+
     private void testRecord(Http2Headers.DynamicTable table,
                             int index,
                             String expectedName,
diff --git a/tests/integration/h2spec/pom.xml b/tests/integration/h2spec/pom.xml
@@ -103,7 +103,7 @@
                     </excludes>
                     <systemPropertyVariables>
                         <java.util.logging.config.file>
-                            ${project.build.outputDirectory}/logging.properties
+                            ${project.build.testOutputDirectory}/logging-test.properties
                         </java.util.logging.config.file>
                     </systemPropertyVariables>
                 </configuration>
@@ -114,7 +114,7 @@
                 <configuration>
                     <systemPropertyVariables>
                         <java.util.logging.config.file>
-                            ${project.build.outputDirectory}/logging.properties
+                            ${project.build.testOutputDirectory}/logging-test.properties
                         </java.util.logging.config.file>
                     </systemPropertyVariables>
                     <redirectTestOutputToFile>${redirectTestOutputToFile}</redirectTestOutputToFile>
diff --git a/tests/integration/h2spec/src/test/java/H2SpecIT.java b/tests/integration/h2spec/src/test/java/H2SpecIT.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024 Oracle and/or its affiliates.
+ * Copyright (c) 2024, 2025 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -22,6 +22,7 @@
 
 import javax.xml.parsers.DocumentBuilderFactory;
 
+import io.helidon.logging.common.LogConfig;
 import io.helidon.webserver.WebServer;
 import io.helidon.webserver.http.HttpRouting;
 import io.helidon.webserver.http2.Http2Config;
@@ -35,6 +36,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testcontainers.containers.GenericContainer;
+import org.testcontainers.containers.output.Slf4jLogConsumer;
 import org.testcontainers.containers.wait.strategy.Wait;
 import org.testcontainers.images.PullPolicy;
 import org.testcontainers.images.builder.ImageFromDockerfile;
@@ -48,6 +50,10 @@
 @Testcontainers(disabledWithoutDocker = true)
 class H2SpecIT {
 
+    static {
+        LogConfig.configureRuntime();
+    }
+
     private static final Logger LOGGER = LoggerFactory.getLogger(H2SpecIT.class);
 
     @ParameterizedTest(name = "{0}: {1}")
@@ -88,8 +94,8 @@ private static Stream<Arguments> runH2Spec() {
         try (var cont = new GenericContainer<>(
                 new ImageFromDockerfile().withDockerfile(Path.of("./Dockerfile")))
                 .withAccessToHost(true)
+                .withLogConsumer(new Slf4jLogConsumer(LoggerFactory.getLogger("h2spec")))
                 .withImagePullPolicy(PullPolicy.ageBased(Duration.ofDays(365)))
-                .withLogConsumer(outputFrame -> LOGGER.info(outputFrame.getUtf8StringWithoutLineEnding()))
                 .waitingFor(Wait.forLogMessage(".*Finished in.*", 1))) {
 
             org.testcontainers.Testcontainers.exposeHostPorts(port);
diff --git a/tests/integration/h2spec/src/test/resources/logging-test.properties b/tests/integration/h2spec/src/test/resources/logging-test.properties
@@ -0,0 +1,17 @@
+#
+# Copyright (c) 2025 Oracle and/or its affiliates.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+handlers = java.util.logging.ConsoleHandler
+java.util.logging.ConsoleHandler.level = INFO
