pjfanning commented on code in PR #1394:
URL: https://github.com/apache/pekko-http/pull/1394#discussion_r4485920934


##########
http-core/src/main/scala/org/apache/pekko/http/javadsl/settings/Http2ClientSettings.scala:
##########
@@ -126,4 +127,36 @@ trait Http2ClientSettings { self: 
scaladsl.settings.Http2ClientSettings.Http2Cli
   def withMaxConnectionBackoff(backoff: Duration): Http2ClientSettings =
     copy(maxConnectionBackoff = backoff.toMillis.millis)
 
+  /**
+   * @since 2.0.0
+   */
+  def getFrameTypeThrottleFrameTypes(): java.util.Set[String] = 
frameTypeThrottleFrameTypes.asJava
+
+  /**
+   * @since 2.0.0
+   */
+  def withFrameTypeThrottleFrameTypes(frameTypes: java.util.Set[String]): 
Http2ClientSettings =
+    copy(frameTypeThrottleFrameTypes = frameTypes.asScala.toSet)
+
+  /**
+   * @since 2.0.0
+   */
+  def getFrameTypeThrottleCost(): Int = frameTypeThrottleCost
+
+  /**
+   * @since 2.0.0
+   */
+  def getFrameTypeThrottleBurst(): Int = frameTypeThrottleBurst
+
+  /**
+   * @since 2.0.0
+   */
+  def getFrameTypeThrottleInterval: Duration = 
Duration.ofMillis(frameTypeThrottleInterval.toMillis)
+
+  /**
+   * @since 2.0.0
+   */

Review Comment:
   Added `withFrameTypeThrottleCost` and `withFrameTypeThrottleBurst` to the 
javadsl `Http2ClientSettings` in 41d118a44, and the same setters plus 
`withFrameTypeThrottleFrameTypes` to the javadsl `Http2ServerSettings` in 
fe0a8b469.
   
   Leaving `getFrameTypeThrottleInterval` without `()`: it matches the other 
`Duration` getters in this trait, such as `getPingInterval` and 
`getCompletionTimeout`, and the server's getter of the same name.



##########
http-core/src/main/scala/org/apache/pekko/http/javadsl/settings/Http2ClientSettings.scala:
##########
@@ -126,4 +127,36 @@ trait Http2ClientSettings { self: 
scaladsl.settings.Http2ClientSettings.Http2Cli
   def withMaxConnectionBackoff(backoff: Duration): Http2ClientSettings =
     copy(maxConnectionBackoff = backoff.toMillis.millis)
 
+  /**
+   * @since 2.0.0
+   */
+  def getFrameTypeThrottleFrameTypes(): java.util.Set[String] = 
frameTypeThrottleFrameTypes.asJava
+
+  /**
+   * @since 2.0.0
+   */
+  def withFrameTypeThrottleFrameTypes(frameTypes: java.util.Set[String]): 
Http2ClientSettings =
+    copy(frameTypeThrottleFrameTypes = frameTypes.asScala.toSet)
+
+  /**
+   * @since 2.0.0
+   */
+  def getFrameTypeThrottleCost(): Int = frameTypeThrottleCost
+
+  /**
+   * @since 2.0.0
+   */
+  def getFrameTypeThrottleBurst(): Int = frameTypeThrottleBurst
+
+  /**
+   * @since 2.0.0
+   */
+  def getFrameTypeThrottleInterval: Duration = 
Duration.ofMillis(frameTypeThrottleInterval.toMillis)
+
+  /**
+   * @since 2.0.0
+   */

Review Comment:
   Added `withFrameTypeThrottleCost` and `withFrameTypeThrottleBurst` to the 
javadsl `Http2ClientSettings` in 41d118a44, and the same setters plus 
`withFrameTypeThrottleFrameTypes` to the javadsl `Http2ServerSettings` in 
fe0a8b469.
   
   Leaving `getFrameTypeThrottleInterval` without `()`: it matches the other 
`Duration` getters in this trait, such as `getPingInterval` and 
`getCompletionTimeout`, and the server's getter of the same name.



##########
http-core/src/main/scala/org/apache/pekko/http/scaladsl/settings/Http2ServerSettings.scala:
##########
@@ -435,6 +484,7 @@ object Http2ClientSettings extends 
SettingsCompanion[Http2ClientSettings] {
       "persistent-connection-max-age-jitter must be >= 0 and < 1")
     require(completionTimeout > Duration.Zero, "completion-timeout must be > 
0")
     require(baseConnectionBackoff <= maxConnectionBackoff, 
"base-connection-backoff must be <= max-connection-backoff")

Review Comment:
   Fixed in 41d118a44: the client settings now require 
`frame-type-throttle.cost > 0` and `burst >= 0`, and reference.conf notes the 
limits. fe0a8b469 adds the same checks to the server settings.



##########
http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ClientSpec.scala:
##########
@@ -836,6 +837,52 @@ class Http2ClientSpec extends PekkoSpecWithMaterializer("""
       })
     }
 
+    "throttle frame types" should {
+      val throttleExceeded = "HTTP2 connection failed with error [Maximum 
throttle throughput exceeded.]"
+
+      abstract class WaitingForResponseData extends TestSetup with NetProbes {
+        user.emitRequest(Get("/"))
+        val streamId = network.expect[HeadersFrame]().streamId
+        network.sendHEADERS(streamId, endStream = false, 
Seq(RawHeader(":status", "200")))
+        val entityDataIn = 
ByteStringSinkProbe(user.expectResponse().entity.dataBytes)
+
+        def emptyDataFrames(count: Int): ByteString =
+          Seq.fill(count)(FrameRenderer.renderFrame(FrameType.DATA, 
ByteFlag.Zero, streamId, ByteString.empty))
+            .reduce(_ ++ _)

Review Comment:
   Leaving this as is. It is a test fixture of at most 1000 frames, and 
`reduce(_ ++ _)` is the idiom the sibling throttle specs already use, for 
example `Http2ServerEnableFrameTypeThrottleSpec`. The helper is never called 
with a count of 0.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to