fix: adjust expiring reset stream limits (#858)

h2 has mechanisms to remember a local reset stream for some time afterward, to support what it says in RFC 9113 SS 5.4.2:

> A RST_STREAM is the last frame that an endpoint can send on a stream. The peer that sends the RST_STREAM frame MUST be prepared to receive any frames that were sent or enqueued for sending by the remote peer. These frames can be ignored, except where they modify connection state (such as the state maintained for field section compression (Section 4.3) or flow control).
>
> Normally, an endpoint SHOULD NOT send more than one RST_STREAM frame for any stream. However, an endpoint MAY send additional RST_STREAM frames if it receives frames on a closed stream after more than a round-trip time. This behavior is permitted to deal with misbehaving implementations.

We don't want to remember the streams for forever, that would cause memory to grow. Nor do we want to accept frames on those streams without limit, since that would waste resources. Thus, we limit how much we remember, but stream count and duration.

This changes the defaults to be something perhaps a little more reasonable:

- 50 streams, a conservative amount of half of the recommending minimum concurrent streams any remote SHOULD support.
- 1 second, since the RFC only suggests about 1 RTT. We don't keep track of RTT, but even p99 RTT is around 250ms, maybe 500ms.

One possibility is having different values for server and client, since servers are usually more interested in constraining resources, but at the same time, many servers also contain clients.

cc #856

Author: seanmonstar

src/client.rs

@@ -923,7 +923,7 @@ impl Builder {
     /// received for that stream will result in a connection level protocol
     /// error, forcing the connection to terminate.
     ///
-    /// The default value is 10.
+    /// The default value is currently 50.
     ///
     /// # Examples
     ///
@@ -968,7 +968,7 @@ impl Builder {
     /// received for that stream will result in a connection level protocol
     /// error, forcing the connection to terminate.
     ///
-    /// The default value is 30 seconds.
+    /// The default value is currently 1 second.
     ///
     /// # Examples
     ///

src/proto/mod.rs

@@ -33,6 +33,10 @@ pub type WindowSize = u32;
 pub const MAX_WINDOW_SIZE: WindowSize = (1 << 31) - 1; // i32::MAX as u32
 pub const DEFAULT_REMOTE_RESET_STREAM_MAX: usize = 20;
 pub const DEFAULT_LOCAL_RESET_COUNT_MAX: usize = 1024;
-pub const DEFAULT_RESET_STREAM_MAX: usize = 10;
-pub const DEFAULT_RESET_STREAM_SECS: u64 = 30;
+// RFC 9113 suggests allowing at minimum 100 streams, it seems reasonable to
+// by default allow a portion of that to be remembered as reset for some time.
+pub const DEFAULT_RESET_STREAM_MAX: usize = 50;
+// RFC 9113#5.4.2 suggests ~1 RTT. We don't track that closely, but use a
+// reasonable guess of the average here.
+pub const DEFAULT_RESET_STREAM_SECS: u64 = 1;
 pub const DEFAULT_MAX_SEND_BUFFER_SIZE: usize = 1024 * 400;

src/proto/streams/recv.rs

@@ -912,11 +912,15 @@ impl Recv {
             return;
         }
 
-        tracing::trace!("enqueue_reset_expiration; {:?}", stream.id);
-
         if counts.can_inc_num_reset_streams() {
             counts.inc_num_reset_streams();
+            tracing::trace!("enqueue_reset_expiration; added {:?}", stream.id);
             self.pending_reset_expired.push(stream);
+        } else {
+            tracing::trace!(
+                "enqueue_reset_expiration; dropped {:?}, over max_concurrent_reset_streams",
+                stream.id
+            );
         }
     }
 

src/server.rs

@@ -868,7 +868,7 @@ impl Builder {
     /// received for that stream will result in a connection level protocol
     /// error, forcing the connection to terminate.
     ///
-    /// The default value is 10.
+    /// The default value is currently 50.
     ///
     /// # Examples
     ///
@@ -993,7 +993,7 @@ impl Builder {
     /// received for that stream will result in a connection level protocol
     /// error, forcing the connection to terminate.
     ///
-    /// The default value is 30 seconds.
+    /// The default value is currently 1 second.
     ///
     /// # Examples
     ///