[testing] Adjust port selection algorithm on RBE (#33429)

I've got a hypothesis that we're losing isolation between test shards
right now for "some reason".

This is a change to reflect test sharding in the port distribution that
we use, in an attempt to alleviate that.

---------

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

CMakeLists.txt

@@ -1318,6 +1318,8 @@ if(gRPC_BUILD_TESTS)
     add_dependencies(buildtests_cxx tcp_server_posix_test)
   endif()
   add_dependencies(buildtests_cxx tcp_socket_utils_test)
+  add_dependencies(buildtests_cxx test_config_sharded_test)
+  add_dependencies(buildtests_cxx test_config_unsharded_test)
   add_dependencies(buildtests_cxx test_core_channel_channelz_test)
   add_dependencies(buildtests_cxx test_core_end2end_channelz_test)
   add_dependencies(buildtests_cxx test_core_event_engine_posix_timer_heap_test)
@@ -24653,6 +24655,80 @@ target_link_libraries(tcp_socket_utils_test
 )
 
 
+endif()
+if(gRPC_BUILD_TESTS)
+
+add_executable(test_config_sharded_test
+  test/core/util/test_config_sharded_test.cc
+  third_party/googletest/googletest/src/gtest-all.cc
+  third_party/googletest/googlemock/src/gmock-all.cc
+)
+target_compile_features(test_config_sharded_test PUBLIC cxx_std_14)
+target_include_directories(test_config_sharded_test
+  PRIVATE
+    ${CMAKE_CURRENT_SOURCE_DIR}
+    ${CMAKE_CURRENT_SOURCE_DIR}/include
+    ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR}
+    ${_gRPC_RE2_INCLUDE_DIR}
+    ${_gRPC_SSL_INCLUDE_DIR}
+    ${_gRPC_UPB_GENERATED_DIR}
+    ${_gRPC_UPB_GRPC_GENERATED_DIR}
+    ${_gRPC_UPB_INCLUDE_DIR}
+    ${_gRPC_XXHASH_INCLUDE_DIR}
+    ${_gRPC_ZLIB_INCLUDE_DIR}
+    third_party/googletest/googletest/include
+    third_party/googletest/googletest
+    third_party/googletest/googlemock/include
+    third_party/googletest/googlemock
+    ${_gRPC_PROTO_GENS_DIR}
+)
+
+target_link_libraries(test_config_sharded_test
+  ${_gRPC_BASELIB_LIBRARIES}
+  ${_gRPC_PROTOBUF_LIBRARIES}
+  ${_gRPC_ZLIB_LIBRARIES}
+  ${_gRPC_ALLTARGETS_LIBRARIES}
+  grpc_test_util
+)
+
+
+endif()
+if(gRPC_BUILD_TESTS)
+
+add_executable(test_config_unsharded_test
+  test/core/util/test_config_unsharded_test.cc
+  third_party/googletest/googletest/src/gtest-all.cc
+  third_party/googletest/googlemock/src/gmock-all.cc
+)
+target_compile_features(test_config_unsharded_test PUBLIC cxx_std_14)
+target_include_directories(test_config_unsharded_test
+  PRIVATE
+    ${CMAKE_CURRENT_SOURCE_DIR}
+    ${CMAKE_CURRENT_SOURCE_DIR}/include
+    ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR}
+    ${_gRPC_RE2_INCLUDE_DIR}
+    ${_gRPC_SSL_INCLUDE_DIR}
+    ${_gRPC_UPB_GENERATED_DIR}
+    ${_gRPC_UPB_GRPC_GENERATED_DIR}
+    ${_gRPC_UPB_INCLUDE_DIR}
+    ${_gRPC_XXHASH_INCLUDE_DIR}
+    ${_gRPC_ZLIB_INCLUDE_DIR}
+    third_party/googletest/googletest/include
+    third_party/googletest/googletest
+    third_party/googletest/googlemock/include
+    third_party/googletest/googlemock
+    ${_gRPC_PROTO_GENS_DIR}
+)
+
+target_link_libraries(test_config_unsharded_test
+  ${_gRPC_BASELIB_LIBRARIES}
+  ${_gRPC_PROTOBUF_LIBRARIES}
+  ${_gRPC_ZLIB_LIBRARIES}
+  ${_gRPC_ALLTARGETS_LIBRARIES}
+  grpc_test_util
+)
+
+
 endif()
 if(gRPC_BUILD_TESTS)
 

build_autogenerated.yaml

@@ -14681,6 +14681,24 @@ targets:
   deps:
   - grpc
   uses_polling: false
+- name: test_config_sharded_test
+  gtest: true
+  build: test
+  language: c++
+  headers: []
+  src:
+  - test/core/util/test_config_sharded_test.cc
+  deps:
+  - grpc_test_util
+- name: test_config_unsharded_test
+  gtest: true
+  build: test
+  language: c++
+  headers: []
+  src:
+  - test/core/util/test_config_unsharded_test.cc
+  deps:
+  - grpc_test_util
 - name: test_core_channel_channelz_test
   gtest: true
   build: test

test/core/util/BUILD

@@ -120,6 +120,7 @@ grpc_cc_library(
         "absl/status:statusor",
         "absl/strings",
         "absl/strings:str_format",
+        "absl/types:optional",
     ],
     language = "C++",
     deps = [
@@ -139,6 +140,7 @@ grpc_cc_library(
         "//src/core:channel_args_endpoint_config",
         "//src/core:channel_args_preconditioning",
         "//src/core:closure",
+        "//src/core:env",
         "//src/core:error",
         "//src/core:gpr_atm",
         "//src/core:grpc_sockaddr",
@@ -154,6 +156,21 @@ grpc_cc_library(
     alwayslink = 1,
 )
 
+grpc_cc_test(
+    name = "test_config_sharded_test",
+    srcs = ["test_config_sharded_test.cc"],
+    external_deps = ["gtest"],
+    shard_count = 2,
+    deps = ["grpc_test_util"],
+)
+
+grpc_cc_test(
+    name = "test_config_unsharded_test",
+    srcs = ["test_config_unsharded_test.cc"],
+    external_deps = ["gtest"],
+    deps = ["grpc_test_util"],
+)
+
 grpc_cc_library(
     name = "grpc_test_util_unsecure",
     srcs = TEST_UTILS_THAT_USE_GRPC_H_SOURCES,
@@ -164,6 +181,7 @@ grpc_cc_library(
         "absl/status:statusor",
         "absl/strings",
         "absl/strings:str_format",
+        "absl/types:optional",
     ],
     language = "C++",
     deps = [
@@ -181,6 +199,7 @@ grpc_cc_library(
         "//src/core:channel_args_endpoint_config",
         "//src/core:channel_args_preconditioning",
         "//src/core:closure",
+        "//src/core:env",
         "//src/core:error",
         "//src/core:gpr_atm",
         "//src/core:grpc_sockaddr",

test/core/util/port_isolated_runtime_environment.cc

@@ -24,6 +24,9 @@
 //
 #include <stdlib.h>
 
+#include <atomic>
+#include <random>
+
 #include <grpc/support/atm.h>
 #include <grpc/support/log.h>
 #include <grpc/support/time.h>
@@ -33,25 +36,36 @@
 #include "test/core/util/port.h"
 #include "test/core/util/test_config.h"
 
-#define MIN_PORT 1025
-#define MAX_PORT 32766
+namespace {
+const int kMinAvailablePort = 1025;
+const int kMaxAvailablePort = 32766;
+const int kPortsPerShard = (kMaxAvailablePort - kMinAvailablePort) /
+                           (grpc::testing::kMaxGtestShard + 1);
+static int MinPortForShard(int shard) {
+  return kMinAvailablePort + kPortsPerShard * shard;
+}
+static int MaxPortForShard(int shard) {
+  return MinPortForShard(shard) + kPortsPerShard - 1;
+}
+const int kCurrentShard = grpc::testing::CurrentGtestShard();
+const int kMinPort = MinPortForShard(kCurrentShard);
+const int kMaxPort = MaxPortForShard(kCurrentShard);
 
-static int get_random_port_offset() {
-  srand(gpr_now(GPR_CLOCK_REALTIME).tv_nsec);
-  double rnd = static_cast<double>(rand()) /
-               (static_cast<double>(RAND_MAX) + 1.0);  // values from [0,1)
-  return static_cast<int>(rnd * (MAX_PORT - MIN_PORT + 1));
+int GetRandomPortOffset() {
+  std::random_device rd;
+  std::uniform_int_distribution<> dist(kMinPort, kMaxPort);
+  return dist(rd);
 }
 
-static int s_initial_offset = get_random_port_offset();
-static gpr_atm s_pick_counter = 0;
+const int kInitialOffset = GetRandomPortOffset();
+std::atomic<int> g_pick_counter{0};
+}  // namespace
 
 static int grpc_pick_unused_port_or_die_impl(void) {
-  int orig_counter_val =
-      static_cast<int>(gpr_atm_full_fetch_add(&s_pick_counter, 1));
-  GPR_ASSERT(orig_counter_val < (MAX_PORT - MIN_PORT + 1));
-  return MIN_PORT +
-         (s_initial_offset + orig_counter_val) % (MAX_PORT - MIN_PORT + 1);
+  int orig_counter_val = g_pick_counter.fetch_add(1, std::memory_order_seq_cst);
+  GPR_ASSERT(orig_counter_val < (kMaxPort - kMinPort + 1));
+  return kMinPort +
+         (kInitialOffset + orig_counter_val) % (kMaxPort - kMinPort + 1);
 }
 
 static int isolated_pick_unused_port_or_die(void) {

test/core/util/test_config.cc

@@ -27,14 +27,17 @@
 #include "absl/debugging/failure_signal_handler.h"
 #include "absl/status/status.h"
 #include "absl/strings/match.h"
+#include "absl/strings/numbers.h"
 #include "absl/strings/str_format.h"
 #include "absl/strings/string_view.h"
+#include "absl/types/optional.h"
 
 #include <grpc/grpc.h>
 #include <grpc/support/log.h>
 #include <grpc/support/time.h>
 
 #include "src/core/lib/gprpp/crash.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"
@@ -155,6 +158,16 @@ bool grpc_wait_until_shutdown(int64_t time_s) {
 namespace grpc {
 namespace testing {
 
+int CurrentGtestShard() {
+  auto env = grpc_core::GetEnv("GTEST_SHARD_INDEX");
+  if (!env.has_value()) return 0;
+  int shard;
+  if (!absl::SimpleAtoi(*env, &shard)) return 0;
+  GPR_ASSERT(shard >= 0);
+  GPR_ASSERT(shard < kMaxGtestShard);
+  return shard + 1;
+}
+
 TestEnvironment::TestEnvironment(int* argc, char** argv) {
   grpc_test_init(argc, argv);
 }

test/core/util/test_config.h

@@ -49,6 +49,12 @@ bool grpc_wait_until_shutdown(int64_t time_s);
 namespace grpc {
 namespace testing {
 
+constexpr int kMaxGtestShard = 50;
+
+// Which gtest shard are we running on?
+// Returns 0..kMaxGtestShard inclusive (0 indicates non sharded)
+int CurrentGtestShard();
+
 // A TestEnvironment object should be alive in the main function of a test. It
 // provides test init and shutdown inside.
 class TestEnvironment {

test/core/util/test_config_sharded_test.cc

@@ -0,0 +1,35 @@
+// Copyright 2023 gRPC authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "gtest/gtest.h"
+
+#include "test/core/util/test_config.h"
+
+namespace grpc {
+namespace test {
+
+TEST(TestConfigTest, CurrentGtestShardIsNonZero) {
+  const int shard = testing::CurrentGtestShard();
+  EXPECT_NE(shard, 0);
+  EXPECT_LE(shard, 50);
+}
+
+}  // namespace test
+}  // namespace grpc
+
+int main(int argc, char** argv) {
+  grpc::testing::TestEnvironment env(&argc, argv);
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

test/core/util/test_config_unsharded_test.cc

@@ -0,0 +1,34 @@
+// Copyright 2023 gRPC authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "gtest/gtest.h"
+
+#include "test/core/util/test_config.h"
+
+namespace grpc {
+namespace test {
+
+TEST(TestConfigTest, CurrentGtestShardZero) {
+  const int shard = testing::CurrentGtestShard();
+  EXPECT_EQ(shard, 0);
+}
+
+}  // namespace test
+}  // namespace grpc
+
+int main(int argc, char** argv) {
+  grpc::testing::TestEnvironment env(&argc, argv);
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

tools/run_tests/generated/tests.json

@@ -9559,6 +9559,54 @@
     ],
     "uses_polling": false
   },
+  {
+    "args": [],
+    "benchmark": false,
+    "ci_platforms": [
+      "linux",
+      "mac",
+      "posix",
+      "windows"
+    ],
+    "cpu_cost": 1.0,
+    "exclude_configs": [],
+    "exclude_iomgrs": [],
+    "flaky": false,
+    "gtest": true,
+    "language": "c++",
+    "name": "test_config_sharded_test",
+    "platforms": [
+      "linux",
+      "mac",
+      "posix",
+      "windows"
+    ],
+    "uses_polling": true
+  },
+  {
+    "args": [],
+    "benchmark": false,
+    "ci_platforms": [
+      "linux",
+      "mac",
+      "posix",
+      "windows"
+    ],
+    "cpu_cost": 1.0,
+    "exclude_configs": [],
+    "exclude_iomgrs": [],
+    "flaky": false,
+    "gtest": true,
+    "language": "c++",
+    "name": "test_config_unsharded_test",
+    "platforms": [
+      "linux",
+      "mac",
+      "posix",
+      "windows"
+    ],
+    "uses_polling": true
+  },
   {
     "args": [],
     "benchmark": false,