Revert "[client idleness] enable via experiment" (#34675)

Reverts grpc/grpc#34653

src/core/BUILD

@@ -3722,7 +3722,6 @@ grpc_cc_library(
         "closure",
         "error",
         "exec_ctx_wakeup_scheduler",
-        "experiments",
         "http2_errors",
         "idle_filter_state",
         "loop",

src/core/ext/filters/channel_idle/channel_idle_filter.cc

@@ -35,7 +35,6 @@
 #include "src/core/lib/channel/promise_based_filter.h"
 #include "src/core/lib/config/core_configuration.h"
 #include "src/core/lib/debug/trace.h"
-#include "src/core/lib/experiments/experiments.h"
 #include "src/core/lib/gprpp/debug_location.h"
 #include "src/core/lib/gprpp/no_destruct.h"
 #include "src/core/lib/gprpp/orphanable.h"
@@ -60,12 +59,9 @@ namespace grpc_core {
 
 namespace {
 
-// TODO(roth): This can go back to being a constant when the experiment
-// is removed.
-Duration DefaultIdleTimeout() {
-  if (IsClientIdlenessEnabled()) return Duration::Minutes(30);
-  return Duration::Infinity();
-}
+// TODO(ctiller): The idle filter was disabled in client channel by default
+// due to b/143502997. Now the bug is fixed enable the filter by default.
+const auto kDefaultIdleTimeout = Duration::Infinity();
 
 // If these settings change, make sure that we are not sending a GOAWAY for
 // inproc transport, since a GOAWAY to inproc ends up destroying the transport.
@@ -88,7 +84,7 @@ namespace {
 
 Duration GetClientIdleTimeout(const ChannelArgs& args) {
   return args.GetDurationFromIntMillis(GRPC_ARG_CLIENT_IDLE_TIMEOUT_MS)
-      .value_or(DefaultIdleTimeout());
+      .value_or(kDefaultIdleTimeout);
 }
 
 }  // namespace

src/core/lib/experiments/experiments.cc

@@ -41,9 +41,6 @@ const char* const additional_constraints_chttp2_batch_requests = "{}";
 const char* const description_chttp2_offload_on_rst_stream =
     "Offload work on RST_STREAM.";
 const char* const additional_constraints_chttp2_offload_on_rst_stream = "{}";
-const char* const description_client_idleness =
-    "If enabled, client channel idleness is enabled by default.";
-const char* const additional_constraints_client_idleness = "{}";
 const char* const description_client_privacy = "If set, client privacy";
 const char* const additional_constraints_client_privacy = "{}";
 const char* const description_combiner_offload_to_event_engine =
@@ -194,8 +191,6 @@ const ExperimentMetadata g_experiment_metadata[] = {
      additional_constraints_chttp2_batch_requests, true, true},
     {"chttp2_offload_on_rst_stream", description_chttp2_offload_on_rst_stream,
      additional_constraints_chttp2_offload_on_rst_stream, true, true},
-    {"client_idleness", description_client_idleness,
-     additional_constraints_client_idleness, true, true},
     {"client_privacy", description_client_privacy,
      additional_constraints_client_privacy, false, false},
     {"combiner_offload_to_event_engine",
@@ -294,9 +289,6 @@ const char* const additional_constraints_chttp2_batch_requests = "{}";
 const char* const description_chttp2_offload_on_rst_stream =
     "Offload work on RST_STREAM.";
 const char* const additional_constraints_chttp2_offload_on_rst_stream = "{}";
-const char* const description_client_idleness =
-    "If enabled, client channel idleness is enabled by default.";
-const char* const additional_constraints_client_idleness = "{}";
 const char* const description_client_privacy = "If set, client privacy";
 const char* const additional_constraints_client_privacy = "{}";
 const char* const description_combiner_offload_to_event_engine =
@@ -447,8 +439,6 @@ const ExperimentMetadata g_experiment_metadata[] = {
      additional_constraints_chttp2_batch_requests, true, true},
     {"chttp2_offload_on_rst_stream", description_chttp2_offload_on_rst_stream,
      additional_constraints_chttp2_offload_on_rst_stream, true, true},
-    {"client_idleness", description_client_idleness,
-     additional_constraints_client_idleness, true, true},
     {"client_privacy", description_client_privacy,
      additional_constraints_client_privacy, false, false},
     {"combiner_offload_to_event_engine",
@@ -547,9 +537,6 @@ const char* const additional_constraints_chttp2_batch_requests = "{}";
 const char* const description_chttp2_offload_on_rst_stream =
     "Offload work on RST_STREAM.";
 const char* const additional_constraints_chttp2_offload_on_rst_stream = "{}";
-const char* const description_client_idleness =
-    "If enabled, client channel idleness is enabled by default.";
-const char* const additional_constraints_client_idleness = "{}";
 const char* const description_client_privacy = "If set, client privacy";
 const char* const additional_constraints_client_privacy = "{}";
 const char* const description_combiner_offload_to_event_engine =
@@ -700,8 +687,6 @@ const ExperimentMetadata g_experiment_metadata[] = {
      additional_constraints_chttp2_batch_requests, true, true},
     {"chttp2_offload_on_rst_stream", description_chttp2_offload_on_rst_stream,
      additional_constraints_chttp2_offload_on_rst_stream, true, true},
-    {"client_idleness", description_client_idleness,
-     additional_constraints_client_idleness, true, true},
     {"client_privacy", description_client_privacy,
      additional_constraints_client_privacy, false, false},
     {"combiner_offload_to_event_engine",

src/core/lib/experiments/experiments.h

@@ -74,8 +74,6 @@ inline bool IsCanaryClientPrivacyEnabled() { return false; }
 inline bool IsChttp2BatchRequestsEnabled() { return true; }
 #define GRPC_EXPERIMENT_IS_INCLUDED_CHTTP2_OFFLOAD_ON_RST_STREAM
 inline bool IsChttp2OffloadOnRstStreamEnabled() { return true; }
-#define GRPC_EXPERIMENT_IS_INCLUDED_CLIENT_IDLENESS
-inline bool IsClientIdlenessEnabled() { return true; }
 inline bool IsClientPrivacyEnabled() { return false; }
 #define GRPC_EXPERIMENT_IS_INCLUDED_COMBINER_OFFLOAD_TO_EVENT_ENGINE
 inline bool IsCombinerOffloadToEventEngineEnabled() { return true; }
@@ -141,8 +139,6 @@ inline bool IsCanaryClientPrivacyEnabled() { return false; }
 inline bool IsChttp2BatchRequestsEnabled() { return true; }
 #define GRPC_EXPERIMENT_IS_INCLUDED_CHTTP2_OFFLOAD_ON_RST_STREAM
 inline bool IsChttp2OffloadOnRstStreamEnabled() { return true; }
-#define GRPC_EXPERIMENT_IS_INCLUDED_CLIENT_IDLENESS
-inline bool IsClientIdlenessEnabled() { return true; }
 inline bool IsClientPrivacyEnabled() { return false; }
 #define GRPC_EXPERIMENT_IS_INCLUDED_COMBINER_OFFLOAD_TO_EVENT_ENGINE
 inline bool IsCombinerOffloadToEventEngineEnabled() { return true; }
@@ -208,8 +204,6 @@ inline bool IsCanaryClientPrivacyEnabled() { return false; }
 inline bool IsChttp2BatchRequestsEnabled() { return true; }
 #define GRPC_EXPERIMENT_IS_INCLUDED_CHTTP2_OFFLOAD_ON_RST_STREAM
 inline bool IsChttp2OffloadOnRstStreamEnabled() { return true; }
-#define GRPC_EXPERIMENT_IS_INCLUDED_CLIENT_IDLENESS
-inline bool IsClientIdlenessEnabled() { return true; }
 inline bool IsClientPrivacyEnabled() { return false; }
 #define GRPC_EXPERIMENT_IS_INCLUDED_COMBINER_OFFLOAD_TO_EVENT_ENGINE
 inline bool IsCombinerOffloadToEventEngineEnabled() { return true; }
@@ -265,7 +259,6 @@ enum ExperimentIds {
   kExperimentIdCanaryClientPrivacy,
   kExperimentIdChttp2BatchRequests,
   kExperimentIdChttp2OffloadOnRstStream,
-  kExperimentIdClientIdleness,
   kExperimentIdClientPrivacy,
   kExperimentIdCombinerOffloadToEventEngine,
   kExperimentIdEventEngineClient,
@@ -323,10 +316,6 @@ inline bool IsChttp2BatchRequestsEnabled() {
 inline bool IsChttp2OffloadOnRstStreamEnabled() {
   return IsExperimentEnabled(kExperimentIdChttp2OffloadOnRstStream);
 }
-#define GRPC_EXPERIMENT_IS_INCLUDED_CLIENT_IDLENESS
-inline bool IsClientIdlenessEnabled() {
-  return IsExperimentEnabled(kExperimentIdClientIdleness);
-}
 #define GRPC_EXPERIMENT_IS_INCLUDED_CLIENT_PRIVACY
 inline bool IsClientPrivacyEnabled() {
   return IsExperimentEnabled(kExperimentIdClientPrivacy);

src/core/lib/experiments/experiments.yaml

@@ -65,11 +65,6 @@
   expiry: 2024/03/03
   owner: ctiller@google.com
   test_tags: ["cpp_end2end_test", "flow_control_test"]
-- name: client_idleness
-  description: If enabled, client channel idleness is enabled by default.
-  expiry: 2023/12/15
-  owner: roth@google.com
-  test_tags: []
 - name: client_privacy
   description:
     If set, client privacy

src/core/lib/experiments/rollouts.yaml

@@ -46,8 +46,6 @@
   default: true
 - name: chttp2_offload_on_rst_stream
   default: true
-- name: client_idleness
-  default: true
 - name: client_privacy
   default: false
 - name: combiner_offload_to_event_engine

test/core/channel/minimal_stack_is_minimal_test.cc

@@ -134,7 +134,7 @@ TEST(ChannelStackFilters, LooksAsExpected) {
                                       "http-server", "compression",
                                       "server_call_tracer", "connected"}));
   EXPECT_EQ(MakeStack(nullptr, no_args, GRPC_CLIENT_CHANNEL),
-            std::vector<std::string>({"client_idle", "client-channel"}));
+            std::vector<std::string>({"client-channel"}));
 }
 
 int main(int argc, char** argv) {