[build] Fixes for experiments, poll strategy (#31042)

* fix

* Automated change: Fix sanity tests

* fix

* fix

* fix

* fix

* fix

Co-authored-by: ctiller <ctiller@users.noreply.github.com>

bazel/experiments.bzl

@@ -43,7 +43,6 @@ EXPERIMENTS = {
         ],
         "resource_quota_test": [
             "memory_pressure_controller",
-            "periodic_resource_quota_reclamation",
             "unconstrained_max_quota_buffer_size",
         ],
     },

bazel/grpc_build_system.bzl

@@ -288,6 +288,7 @@ def expand_tests(name, srcs, deps, tags, args, exclude_pollers, uses_polling, us
             "deps": deps,
             "tags": tags,
             "args": args,
+            "env": {},
         })
     else:
         # On linux we run the same test with the default EventEngine, once for each
@@ -304,7 +305,10 @@ def expand_tests(name, srcs, deps, tags, args, exclude_pollers, uses_polling, us
                     "no_mac",
                     "bazel_only",
                 ]),
-                "args": args + ["--poller=" + poller],
+                "args": args,
+                "env": {
+                    "GRPC_POLL_STRATEGY": poller,
+                },
             })
 
         # Now generate one test for each subsequent EventEngine, all using the
@@ -320,6 +324,7 @@ def expand_tests(name, srcs, deps, tags, args, exclude_pollers, uses_polling, us
                 "deps": deps,
                 "tags": tags + ["no_linux"],
                 "args": args,
+                "env": {},
             })
         else:
             for engine_name, engine in EVENT_ENGINES.items():
@@ -338,6 +343,7 @@ def expand_tests(name, srcs, deps, tags, args, exclude_pollers, uses_polling, us
                     "deps": deps,
                     "tags": test_tags,
                     "args": test_args,
+                    "env": {},
                 })
 
     experiments = {}
@@ -354,8 +360,8 @@ def expand_tests(name, srcs, deps, tags, args, exclude_pollers, uses_polling, us
         # format: <mode>: (enabled_target_tags, disabled_target_tags)
         "dbg": (["noopt"], ["nodbg"]),
         "opt": (["nodbg"], ["noopt"]),
-        "on": ([], None),
-        "off": (None, []),
+        "on": (None, []),
+        "off": ([], None),
     }
 
     must_have_tags = [
@@ -376,7 +382,9 @@ def expand_tests(name, srcs, deps, tags, args, exclude_pollers, uses_polling, us
                 for config in poller_config:
                     config = dict(config)
                     config["name"] = config["name"] + "@experiment=" + experiment
-                    config["args"] = config["args"] + ["--experiment=" + experiment]
+                    env = dict(config["env"])
+                    env["GRPC_EXPERIMENTS"] = experiment
+                    config["env"] = env
                     tags = config["tags"]
                     for tag in must_have_tags + enabled_tags:
                         if tag not in tags:
@@ -388,7 +396,9 @@ def expand_tests(name, srcs, deps, tags, args, exclude_pollers, uses_polling, us
                 for config in poller_config:
                     config = dict(config)
                     config["name"] = config["name"] + "@experiment=no_" + experiment
-                    config["args"] = config["args"] + ["--experiment=-" + experiment]
+                    env = dict(config["env"])
+                    env["GRPC_EXPERIMENTS"] = "-" + experiment
+                    config["env"] = env
                     tags = config["tags"]
                     for tag in must_have_tags + disabled_tags:
                         if tag not in tags:
@@ -460,6 +470,7 @@ def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data
             deps = poller_config["deps"],
             tags = poller_config["tags"],
             args = poller_config["args"],
+            env = poller_config["env"],
             **test_args
         )
 
@@ -553,6 +564,7 @@ def grpc_sh_test(name, srcs = [], args = [], data = [], uses_polling = True, siz
             deps = poller_config["deps"],
             tags = poller_config["tags"],
             args = poller_config["args"],
+            env = poller_config["env"],
             **test_args
         )
 

src/core/lib/experiments/config.cc

@@ -117,7 +117,7 @@ void PrintExperimentsList() {
                                 strlen(g_experiment_metadata[i].name) + 1,
                             ' '),
                 IsExperimentEnabled(i) ? "ON " : "OFF", " (default:",
-                g_experiment_metadata->default_value ? "ON" : "OFF",
+                g_experiment_metadata[i].default_value ? "ON" : "OFF",
                 g_forced_experiments[i].forced
                     ? absl::StrCat(" force:",
                                    g_forced_experiments[i].value ? "ON" : "OFF")

src/core/lib/experiments/experiments.yaml

@@ -16,6 +16,7 @@
 # name:        name of the experiment
 # description: description of the experiment
 # default:     one of:
+#              - broken  - the experiment defaults to off and is not tested on
 #              - false   - the experiment defaults to off
 #              - debug   - the experiment defaults to on in debug,
 #                          off in release
@@ -28,10 +29,12 @@
 #              that that test should be run with this experiment enabled in CI
 #
 # Well known test tags:
-#   core_end2end_tests: all tests, fixtures in the core end2end suite
-#   endpoint_test:      endpoint related iomgr tests
-#   flow_control_test:  tests pertaining explicitly to flow control
-#   hpack_test:         hpack encode/decode tests
+#   core_end2end_tests:  all tests, fixtures in the core end2end suite
+#   endpoint_test:       endpoint related iomgr tests
+#   flow_control_test:   tests pertaining explicitly to flow control
+#   hpack_test:          hpack encode/decode tests
+#   promise_test:        tests around the promise architecture
+#   resource_quota_test: tests known to exercse resource quota
 
 - name: tcp_frame_size_tuning
   description:
@@ -84,10 +87,10 @@
 - name: periodic_resource_quota_reclamation
   description:
     Periodically return memory to the resource quota
-  default: false
+  default: broken
   expiry: 2022/10/01
   owner: ctiller@google.com
-  test_tags: [resource_quota_test]
+  test_tags: [resource_quota_test, promise_test, hpack_test]
 - name: unconstrained_max_quota_buffer_size
   description:
     Discard the cap on the max free pool size for one memory allocator

test/core/gprpp/BUILD

@@ -306,6 +306,9 @@ grpc_cc_test(
     srcs = ["chunked_vector_test.cc"],
     external_deps = ["gtest"],
     language = "c++",
+    tags = [
+        "resource_quota_test",
+    ],
     uses_event_engine = False,
     uses_polling = False,
     deps = [

test/core/promise/BUILD

@@ -83,6 +83,7 @@ grpc_cc_test(
     external_deps = ["gtest"],
     language = "c++",
     tags = [
+        "promise_test",
         "resource_quota_test",
     ],
     uses_event_engine = False,
@@ -105,6 +106,7 @@ grpc_cc_test(
     srcs = ["map_test.cc"],
     external_deps = ["gtest"],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -118,6 +120,7 @@ grpc_cc_test(
     srcs = ["race_test.cc"],
     external_deps = ["gtest"],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -133,6 +136,7 @@ grpc_cc_test(
         "absl/functional:bind_front",
     ],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -146,6 +150,7 @@ grpc_cc_test(
     srcs = ["if_test.cc"],
     external_deps = ["gtest"],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -158,6 +163,7 @@ grpc_cc_test(
     srcs = ["loop_test.cc"],
     external_deps = ["gtest"],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -171,6 +177,7 @@ grpc_cc_test(
     srcs = ["join_test.cc"],
     external_deps = ["gtest"],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -183,6 +190,7 @@ grpc_cc_test(
     srcs = ["try_join_test.cc"],
     external_deps = ["gtest"],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -195,6 +203,7 @@ grpc_cc_test(
     srcs = ["seq_test.cc"],
     external_deps = ["gtest"],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -207,6 +216,7 @@ grpc_cc_test(
     srcs = ["try_seq_test.cc"],
     external_deps = ["gtest"],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -219,6 +229,7 @@ grpc_cc_test(
     srcs = ["try_seq_metadata_test.cc"],
     external_deps = ["gtest"],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -234,6 +245,7 @@ grpc_cc_test(
     external_deps = ["gtest"],
     flaky = True,
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -251,6 +263,7 @@ grpc_cc_test(
     srcs = ["latch_test.cc"],
     external_deps = ["gtest"],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -266,6 +279,7 @@ grpc_cc_test(
     srcs = ["observable_test.cc"],
     external_deps = ["gtest"],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -281,6 +295,7 @@ grpc_cc_test(
     srcs = ["for_each_test.cc"],
     external_deps = ["gtest"],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -300,6 +315,7 @@ grpc_cc_test(
     srcs = ["pipe_test.cc"],
     external_deps = ["gtest"],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -336,6 +352,7 @@ grpc_cc_test(
     srcs = ["exec_ctx_wakeup_scheduler_test.cc"],
     external_deps = ["gtest"],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -352,6 +369,7 @@ grpc_cc_test(
         "absl/synchronization",
     ],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [
@@ -371,6 +389,7 @@ grpc_cc_test(
         "absl/status",
     ],
     language = "c++",
+    tags = ["promise_test"],
     uses_event_engine = False,
     uses_polling = False,
     deps = [

test/core/util/BUILD

@@ -126,7 +126,6 @@ grpc_cc_library(
         "stack_tracer",
         "//:channel_args_preconditioning",
         "//:config",
-        "//:env",
         "//:gpr",
         "//:gpr_atm",
         "//:grpc",
@@ -166,7 +165,6 @@ grpc_cc_library(
         "stack_tracer",
         "//:channel_args_preconditioning",
         "//:config",
-        "//:env",
         "//:gpr",
         "//:gpr_atm",
         "//:grpc_security_base",

test/core/util/test_config.cc

@@ -32,7 +32,6 @@
 #include <grpc/support/log.h>
 #include <grpc/support/time.h>
 
-#include "src/core/lib/gprpp/env.h"
 #include "src/core/lib/surface/init.h"
 #include "test/core/event_engine/test_init.h"
 #include "test/core/util/build.h"
@@ -102,17 +101,9 @@ void RmArg(int i, int* argc, char** argv) {
 void ParseTestArgs(int* argc, char** argv) {
   if (argc == nullptr || *argc <= 1) return;
   // flags to look for and consume
-  const absl::string_view poller_flag{"--poller="};
   const absl::string_view engine_flag{"--engine="};
-  const absl::string_view experiment_flag{"--experiment="};
   int i = 1;
   while (i < *argc) {
-    if (absl::StartsWith(argv[i], poller_flag)) {
-      grpc_core::SetEnv("GRPC_POLL_STRATEGY", argv[i] + poller_flag.length());
-      // remove the spent argv
-      RmArg(i, argc, argv);
-      continue;
-    }
     if (absl::StartsWith(argv[i], engine_flag)) {
       absl::Status engine_set =
           grpc_event_engine::experimental::InitializeTestingEventEngineFactory(
@@ -125,12 +116,6 @@ void ParseTestArgs(int* argc, char** argv) {
       RmArg(i, argc, argv);
       continue;
     }
-    if (absl::StartsWith(argv[i], experiment_flag)) {
-      grpc_core::SetEnv("GRPC_EXPERIMENTS", argv[i] + experiment_flag.length());
-      // remove the spent argv
-      RmArg(i, argc, argv);
-      continue;
-    }
     ++i;
   }
 }

tools/codegen/core/gen_experiments.py

@@ -37,6 +37,7 @@ with open('src/core/lib/experiments/experiments.yaml') as f:
     attrs = yaml.load(f.read(), Loader=yaml.FullLoader)
 
 DEFAULTS = {
+    'broken': 'false',
     False: 'false',
     True: 'true',
     'debug': 'kDefaultForDebugOnly',
@@ -44,6 +45,7 @@ DEFAULTS = {
 }
 
 BZL_LIST_FOR_DEFAULTS = {
+    'broken': None,
     False: 'off',
     True: 'on',
     'debug': 'dbg',
@@ -217,7 +219,8 @@ with open('src/core/lib/experiments/experiments.cc', 'w') as C:
     print("}  // namespace grpc_core", file=C)
 
 bzl_to_tags_to_experiments = dict((key, collections.defaultdict(list))
-                                  for key in BZL_LIST_FOR_DEFAULTS.keys())
+                                  for key in BZL_LIST_FOR_DEFAULTS.keys()
+                                  if key is not None)
 
 for attr in attrs:
     for tag in attr['test_tags']:
@@ -237,7 +240,8 @@ with open('bazel/experiments.bzl', 'w') as B:
 
     bzl_to_tags_to_experiments = sorted(
         (BZL_LIST_FOR_DEFAULTS[default], tags_to_experiments)
-        for default, tags_to_experiments in bzl_to_tags_to_experiments.items())
+        for default, tags_to_experiments in bzl_to_tags_to_experiments.items()
+        if BZL_LIST_FOR_DEFAULTS[default] is not None)
 
     print(file=B)
     print("EXPERIMENTS = {", file=B)