Chiebot-Mirror
/
grpc
8
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

[chttp2] Fix behavior on max concurrent streams exceeded (#34654)

Browse Source 
Noticed that our behavior conflicts with what's mandated:
https://www.rfc-editor.org/rfc/rfc9113.html#section-5.1.2
So here's a quick fix - I like it because it's not a disconnection,
which always feels like one outage avoided.

---------

Co-authored-by: ctiller <ctiller@users.noreply.github.com>
pull/34436/head
Craig Tiller 1 year ago committed by GitHub
parent 002b401430
commit f832772d89
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 59 additions and 1 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 9
      bazel/experiments.bzl
  2. 12
      src/core/ext/transport/chttp2/transport/parsing.cc
  3. 21
      src/core/lib/experiments/experiments.cc
  4. 8
      src/core/lib/experiments/experiments.h
  5. 8
      src/core/lib/experiments/experiments.yaml
  6. 2
      src/core/lib/experiments/rollouts.yaml

9
bazel/experiments.bzl generated
Unescape View File

@ -21,6 +21,9 @@ EXPERIMENTS = {
"dbg": {
},
"off": {
"bad_client_test": [
"rfc_max_concurrent_streams",
],
"core_end2end_test": [
"event_engine_listener",
"promise_based_client_call",
@ -105,6 +108,9 @@ EXPERIMENTS = {
"dbg": {
},
"off": {
"bad_client_test": [
"rfc_max_concurrent_streams",
],
"core_end2end_test": [
"event_engine_listener",
"promise_based_client_call",
@ -189,6 +195,9 @@ EXPERIMENTS = {
"dbg": {
},
"off": {
"bad_client_test": [
"rfc_max_concurrent_streams",
],
"cancel_ares_query_test": [
"event_engine_dns",
],

12
src/core/ext/transport/chttp2/transport/parsing.cc
Unescape View File

@ -650,7 +650,17 @@ static grpc_error_handle init_header_frame_parser(grpc_chttp2_transport* t,
t->stream_map.size() + t->extra_streams >=
t->settings[GRPC_ACKED_SETTINGS]
[GRPC_CHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS])) {
return GRPC_ERROR_CREATE("Max stream count exceeded");
if (grpc_core::IsRfcMaxConcurrentStreamsEnabled()) {
++t->num_pending_induced_frames;
grpc_slice_buffer_add(
&t->qbuf,
grpc_chttp2_rst_stream_create(t->incoming_stream_id,
GRPC_HTTP2_REFUSED_STREAM, nullptr));
grpc_chttp2_initiate_write(t, GRPC_CHTTP2_INITIATE_WRITE_RST_STREAM);
return init_header_skip_frame_parser(t, priority_type, is_eoh);
} else {
return GRPC_ERROR_CREATE("Max stream count exceeded");
}
} else if (GPR_UNLIKELY(
grpc_core::IsRedMaxConcurrentStreamsEnabled() &&
t->stream_map.size() >=

21
src/core/lib/experiments/experiments.cc generated
Unescape View File

@ -117,6 +117,11 @@ const char* const additional_constraints_registered_method_lookup_in_transport =
const char* const description_registered_methods_map =
"Use absl::flat_hash_map for registered methods.";
const char* const additional_constraints_registered_methods_map = "{}";
const char* const description_rfc_max_concurrent_streams =
"If set, enable rfc-compliant behavior (cancellation) in the advent that "
"max concurrent streams are exceeded in chttp2. See "
"https://www.rfc-editor.org/rfc/rfc9113.html#section-5.1.2.";
const char* const additional_constraints_rfc_max_concurrent_streams = "{}";
const char* const description_round_robin_delegate_to_pick_first =
"Change round_robin code to delegate to pick_first as per dualstack "
"backend design.";
@ -250,6 +255,8 @@ const ExperimentMetadata g_experiment_metadata[] = {
additional_constraints_registered_method_lookup_in_transport, true, true},
{"registered_methods_map", description_registered_methods_map,
additional_constraints_registered_methods_map, false, true},
{"rfc_max_concurrent_streams", description_rfc_max_concurrent_streams,
additional_constraints_rfc_max_concurrent_streams, false, true},
{"round_robin_delegate_to_pick_first",
description_round_robin_delegate_to_pick_first,
additional_constraints_round_robin_delegate_to_pick_first, true, true},
@ -385,6 +392,11 @@ const char* const additional_constraints_registered_method_lookup_in_transport =
const char* const description_registered_methods_map =
"Use absl::flat_hash_map for registered methods.";
const char* const additional_constraints_registered_methods_map = "{}";
const char* const description_rfc_max_concurrent_streams =
"If set, enable rfc-compliant behavior (cancellation) in the advent that "
"max concurrent streams are exceeded in chttp2. See "
"https://www.rfc-editor.org/rfc/rfc9113.html#section-5.1.2.";
const char* const additional_constraints_rfc_max_concurrent_streams = "{}";
const char* const description_round_robin_delegate_to_pick_first =
"Change round_robin code to delegate to pick_first as per dualstack "
"backend design.";
@ -518,6 +530,8 @@ const ExperimentMetadata g_experiment_metadata[] = {
additional_constraints_registered_method_lookup_in_transport, true, true},
{"registered_methods_map", description_registered_methods_map,
additional_constraints_registered_methods_map, false, true},
{"rfc_max_concurrent_streams", description_rfc_max_concurrent_streams,
additional_constraints_rfc_max_concurrent_streams, false, true},
{"round_robin_delegate_to_pick_first",
description_round_robin_delegate_to_pick_first,
additional_constraints_round_robin_delegate_to_pick_first, true, true},
@ -653,6 +667,11 @@ const char* const additional_constraints_registered_method_lookup_in_transport =
const char* const description_registered_methods_map =
"Use absl::flat_hash_map for registered methods.";
const char* const additional_constraints_registered_methods_map = "{}";
const char* const description_rfc_max_concurrent_streams =
"If set, enable rfc-compliant behavior (cancellation) in the advent that "
"max concurrent streams are exceeded in chttp2. See "
"https://www.rfc-editor.org/rfc/rfc9113.html#section-5.1.2.";
const char* const additional_constraints_rfc_max_concurrent_streams = "{}";
const char* const description_round_robin_delegate_to_pick_first =
"Change round_robin code to delegate to pick_first as per dualstack "
"backend design.";
@ -786,6 +805,8 @@ const ExperimentMetadata g_experiment_metadata[] = {
additional_constraints_registered_method_lookup_in_transport, true, true},
{"registered_methods_map", description_registered_methods_map,
additional_constraints_registered_methods_map, false, true},
{"rfc_max_concurrent_streams", description_rfc_max_concurrent_streams,
additional_constraints_rfc_max_concurrent_streams, false, true},
{"round_robin_delegate_to_pick_first",
description_round_robin_delegate_to_pick_first,
additional_constraints_round_robin_delegate_to_pick_first, true, true},

8
src/core/lib/experiments/experiments.h generated
Unescape View File

@ -103,6 +103,7 @@ inline bool IsRedMaxConcurrentStreamsEnabled() { return false; }
#define GRPC_EXPERIMENT_IS_INCLUDED_REGISTERED_METHOD_LOOKUP_IN_TRANSPORT
inline bool IsRegisteredMethodLookupInTransportEnabled() { return true; }
inline bool IsRegisteredMethodsMapEnabled() { return false; }
inline bool IsRfcMaxConcurrentStreamsEnabled() { return false; }
#define GRPC_EXPERIMENT_IS_INCLUDED_ROUND_ROBIN_DELEGATE_TO_PICK_FIRST
inline bool IsRoundRobinDelegateToPickFirstEnabled() { return true; }
inline bool IsRstpitEnabled() { return false; }
@ -175,6 +176,7 @@ inline bool IsRedMaxConcurrentStreamsEnabled() { return false; }
#define GRPC_EXPERIMENT_IS_INCLUDED_REGISTERED_METHOD_LOOKUP_IN_TRANSPORT
inline bool IsRegisteredMethodLookupInTransportEnabled() { return true; }
inline bool IsRegisteredMethodsMapEnabled() { return false; }
inline bool IsRfcMaxConcurrentStreamsEnabled() { return false; }
#define GRPC_EXPERIMENT_IS_INCLUDED_ROUND_ROBIN_DELEGATE_TO_PICK_FIRST
inline bool IsRoundRobinDelegateToPickFirstEnabled() { return true; }
inline bool IsRstpitEnabled() { return false; }
@ -247,6 +249,7 @@ inline bool IsRedMaxConcurrentStreamsEnabled() { return false; }
#define GRPC_EXPERIMENT_IS_INCLUDED_REGISTERED_METHOD_LOOKUP_IN_TRANSPORT
inline bool IsRegisteredMethodLookupInTransportEnabled() { return true; }
inline bool IsRegisteredMethodsMapEnabled() { return false; }
inline bool IsRfcMaxConcurrentStreamsEnabled() { return false; }
#define GRPC_EXPERIMENT_IS_INCLUDED_ROUND_ROBIN_DELEGATE_TO_PICK_FIRST
inline bool IsRoundRobinDelegateToPickFirstEnabled() { return true; }
inline bool IsRstpitEnabled() { return false; }
@ -302,6 +305,7 @@ enum ExperimentIds {
kExperimentIdRedMaxConcurrentStreams,
kExperimentIdRegisteredMethodLookupInTransport,
kExperimentIdRegisteredMethodsMap,
kExperimentIdRfcMaxConcurrentStreams,
kExperimentIdRoundRobinDelegateToPickFirst,
kExperimentIdRstpit,
kExperimentIdScheduleCancellationOverWrite,
@ -429,6 +433,10 @@ inline bool IsRegisteredMethodLookupInTransportEnabled() {
inline bool IsRegisteredMethodsMapEnabled() {
return IsExperimentEnabled(kExperimentIdRegisteredMethodsMap);
}
#define GRPC_EXPERIMENT_IS_INCLUDED_RFC_MAX_CONCURRENT_STREAMS
inline bool IsRfcMaxConcurrentStreamsEnabled() {
return IsExperimentEnabled(kExperimentIdRfcMaxConcurrentStreams);
}
#define GRPC_EXPERIMENT_IS_INCLUDED_ROUND_ROBIN_DELEGATE_TO_PICK_FIRST
inline bool IsRoundRobinDelegateToPickFirstEnabled() {
return IsExperimentEnabled(kExperimentIdRoundRobinDelegateToPickFirst);

8
src/core/lib/experiments/experiments.yaml
Unescape View File

@ -205,6 +205,14 @@
expiry: 2024/01/31
owner: alishananda@google.com
test_tags: []
- name: rfc_max_concurrent_streams
description:
If set, enable rfc-compliant behavior (cancellation) in the advent that
max concurrent streams are exceeded in chttp2.
See https://www.rfc-editor.org/rfc/rfc9113.html#section-5.1.2.
expiry: 2024/03/03
owner: ctiller@google.com
test_tags: [bad_client_test]
- name: round_robin_delegate_to_pick_first
description:
Change round_robin code to delegate to pick_first as per dualstack

2
src/core/lib/experiments/rollouts.yaml
Unescape View File

@ -100,6 +100,8 @@
default: true
- name: registered_methods_map
default: false
- name: rfc_max_concurrent_streams
default: false
- name: round_robin_delegate_to_pick_first
default: true
- name: rstpit

Write Preview
Loading…
Cancel
Save