From 8d349df5d3108b4e13f35a7508ca7b94c331b8e4 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Wed, 24 Sep 2025 11:03:34 +0000 Subject: [PATCH 01/19] Moved `XxHash64` library to enable shared use across projects --- settings.gradle | 2 ++ .../zero-allocation-hashing/BUILD.bazel | 26 +++++++++++++++++++ .../zero-allocation-hashing/LICENSE | 0 .../zero-allocation-hashing/NOTICE | 0 .../zero-allocation-hashing/build.gradle | 25 ++++++++++++++++++ .../main/java/io/grpc/tp/zah}/XxHash64.java | 24 ++++++++--------- .../java/io/grpc/tp/zah}/XxHash64Test.java | 2 +- xds/BUILD.bazel | 2 +- xds/build.gradle | 9 +------ .../io/grpc/xds/RingHashLoadBalancer.java | 1 + .../java/io/grpc/xds/XdsNameResolver.java | 1 + .../io/grpc/xds/RingHashLoadBalancerTest.java | 1 + 12 files changed, 71 insertions(+), 22 deletions(-) create mode 100644 third-party/zero-allocation-hashing/BUILD.bazel rename {xds/third_party => third-party}/zero-allocation-hashing/LICENSE (100%) rename {xds/third_party => third-party}/zero-allocation-hashing/NOTICE (100%) create mode 100644 third-party/zero-allocation-hashing/build.gradle rename {xds/third_party/zero-allocation-hashing/main/java/io/grpc/xds => third-party/zero-allocation-hashing/src/main/java/io/grpc/tp/zah}/XxHash64.java (93%) rename {xds/third_party/zero-allocation-hashing/test/java/io/grpc/xds => third-party/zero-allocation-hashing/src/test/java/io/grpc/tp/zah}/XxHash64Test.java (99%) diff --git a/settings.gradle b/settings.gradle index f4df1105090..9e9583bc50b 100644 --- a/settings.gradle +++ b/settings.gradle @@ -93,6 +93,7 @@ include ":grpc-inprocess" include ":grpc-util" include ":grpc-opentelemetry" include ":grpc-context-override-opentelemetry" +include ":grpc-third-party:zero-allocation-hashing" project(':grpc-api').projectDir = "$rootDir/api" as File project(':grpc-core').projectDir = "$rootDir/core" as File @@ -130,6 +131,7 @@ project(':grpc-inprocess').projectDir = "$rootDir/inprocess" as File project(':grpc-util').projectDir = "$rootDir/util" as File project(':grpc-opentelemetry').projectDir = "$rootDir/opentelemetry" as File project(':grpc-context-override-opentelemetry').projectDir = "$rootDir/contextstorage" as File +project(':grpc-third-party:zero-allocation-hashing').projectDir = "$rootDir/third-party/zero-allocation-hashing" as File if (settings.hasProperty('skipCodegen') && skipCodegen.toBoolean()) { println '*** Skipping the build of codegen and compilation of proto files because skipCodegen=true' diff --git a/third-party/zero-allocation-hashing/BUILD.bazel b/third-party/zero-allocation-hashing/BUILD.bazel new file mode 100644 index 00000000000..442f41416c7 --- /dev/null +++ b/third-party/zero-allocation-hashing/BUILD.bazel @@ -0,0 +1,26 @@ +load("@rules_java//java:defs.bzl", "java_binary", "java_library", "java_test") + +java_library( + name = "zero-allocation-hashing", + srcs = [ + "src/main/java/io/grpc/tp/zah/XxHash64.java", + ], + deps = [ + "@maven//:com_google_guava_guava", + ], + visibility = [ + "//xds:__pkg__", + "//util:__pkg__", + ], +) + +java_test( + name = "XxHash64Test", + size = "small", + srcs = ["src/test/java/io/grpc/tp/zah/XxHash64Test.java"], + deps = [ + ":zero-allocation-hashing", + "@maven//:com_google_guava_guava", + "@maven//:junit_junit", + ], +) diff --git a/xds/third_party/zero-allocation-hashing/LICENSE b/third-party/zero-allocation-hashing/LICENSE similarity index 100% rename from xds/third_party/zero-allocation-hashing/LICENSE rename to third-party/zero-allocation-hashing/LICENSE diff --git a/xds/third_party/zero-allocation-hashing/NOTICE b/third-party/zero-allocation-hashing/NOTICE similarity index 100% rename from xds/third_party/zero-allocation-hashing/NOTICE rename to third-party/zero-allocation-hashing/NOTICE diff --git a/third-party/zero-allocation-hashing/build.gradle b/third-party/zero-allocation-hashing/build.gradle new file mode 100644 index 00000000000..2930206d709 --- /dev/null +++ b/third-party/zero-allocation-hashing/build.gradle @@ -0,0 +1,25 @@ +plugins { + id "java-library" +} + +description = 'gRPC: Zero Allocation Hashing' + +dependencies { + implementation libraries.guava + + testImplementation libraries.junit +} + +tasks.named("jar").configure { + manifest { + attributes('Automatic-Module-Name': 'io.grpc.tp.zah') + } +} + +tasks.named("checkstyleMain").configure { + enabled = false +} + +tasks.named("checkstyleTest").configure { + enabled = false +} diff --git a/xds/third_party/zero-allocation-hashing/main/java/io/grpc/xds/XxHash64.java b/third-party/zero-allocation-hashing/src/main/java/io/grpc/tp/zah/XxHash64.java similarity index 93% rename from xds/third_party/zero-allocation-hashing/main/java/io/grpc/xds/XxHash64.java rename to third-party/zero-allocation-hashing/src/main/java/io/grpc/tp/zah/XxHash64.java index 9ca9ebc7762..69f251a1113 100644 --- a/xds/third_party/zero-allocation-hashing/main/java/io/grpc/xds/XxHash64.java +++ b/third-party/zero-allocation-hashing/src/main/java/io/grpc/tp/zah/XxHash64.java @@ -18,7 +18,7 @@ * Modified by the gRPC Authors */ -package io.grpc.xds; +package io.grpc.tp.zah; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; @@ -33,8 +33,8 @@ * * OpenHFT/Zero-Allocation-Hashing. */ -final class XxHash64 { - static final XxHash64 INSTANCE = new XxHash64(0); +final public class XxHash64 { + static public final XxHash64 INSTANCE = new XxHash64(0); // Primes if treated as unsigned private static final long P1 = -7046029288634856825L; @@ -47,12 +47,12 @@ final class XxHash64 { private final long seed; private final long voidHash; - XxHash64(long seed) { + public XxHash64(long seed) { this.seed = seed; this.voidHash = finalize(seed + P5); } - long hashLong(long input) { + public long hashLong(long input) { input = byteOrder == ByteOrder.LITTLE_ENDIAN ? input : Long.reverseBytes(input); long hash = seed + P5 + 8; input *= P2; @@ -63,7 +63,7 @@ long hashLong(long input) { return finalize(hash); } - long hashInt(int input) { + public long hashInt(int input) { input = byteOrder == ByteOrder.LITTLE_ENDIAN ? input : Integer.reverseBytes(input); long hash = seed + P5 + 4; hash ^= (input & 0xFFFFFFFFL) * P1; @@ -71,7 +71,7 @@ long hashInt(int input) { return finalize(hash); } - long hashShort(short input) { + public long hashShort(short input) { input = byteOrder == ByteOrder.LITTLE_ENDIAN ? input : Short.reverseBytes(input); long hash = seed + P5 + 2; hash ^= (input & 0xFFL) * P5; @@ -81,22 +81,22 @@ long hashShort(short input) { return finalize(hash); } - long hashChar(char input) { + public long hashChar(char input) { return hashShort((short) input); } - long hashByte(byte input) { + public long hashByte(byte input) { long hash = seed + P5 + 1; hash ^= (input & 0xFF) * P5; hash = Long.rotateLeft(hash, 11) * P1; return finalize(hash); } - long hashVoid() { + public long hashVoid() { return voidHash; } - long hashAsciiString(String input) { + public long hashAsciiString(String input) { ByteSupplier supplier = new AsciiStringByteSupplier(input); return hashBytes(supplier); } @@ -106,7 +106,7 @@ long hashBytes(byte[] bytes) { return hashBytes(supplier); } - long hashBytes(byte[] bytes, int offset, int len) { + public long hashBytes(byte[] bytes, int offset, int len) { ByteSupplier supplier = new PlainByteSupplier(bytes, offset, len); return hashBytes(supplier); } diff --git a/xds/third_party/zero-allocation-hashing/test/java/io/grpc/xds/XxHash64Test.java b/third-party/zero-allocation-hashing/src/test/java/io/grpc/tp/zah/XxHash64Test.java similarity index 99% rename from xds/third_party/zero-allocation-hashing/test/java/io/grpc/xds/XxHash64Test.java rename to third-party/zero-allocation-hashing/src/test/java/io/grpc/tp/zah/XxHash64Test.java index 10219a2f72e..be14c6351ca 100644 --- a/xds/third_party/zero-allocation-hashing/test/java/io/grpc/xds/XxHash64Test.java +++ b/third-party/zero-allocation-hashing/src/test/java/io/grpc/tp/zah/XxHash64Test.java @@ -18,7 +18,7 @@ * Modified by the gRPC Authors */ -package io.grpc.xds; +package io.grpc.tp.zah; import static org.junit.Assert.assertEquals; diff --git a/xds/BUILD.bazel b/xds/BUILD.bazel index 66c790a654d..1bddcc35d82 100644 --- a/xds/BUILD.bazel +++ b/xds/BUILD.bazel @@ -31,6 +31,7 @@ java_library( "//services:metrics", "//services:metrics_internal", "//stub", + "//third-party/zero-allocation-hashing", "//util", "@com_google_protobuf//:protobuf_java", "@com_google_protobuf//:protobuf_java_util", @@ -73,7 +74,6 @@ java_binary( srcs = glob( [ "src/main/java/**/*.java", - "third_party/zero-allocation-hashing/main/java/**/*.java", ], exclude = ["src/main/java/io/grpc/xds/orca/**"], ), diff --git a/xds/build.gradle b/xds/build.gradle index 8394fe12f6b..6c47a54599d 100644 --- a/xds/build.gradle +++ b/xds/build.gradle @@ -13,9 +13,6 @@ description = "gRPC: XDS plugin" sourceSets { thirdparty { - java { - srcDir "${projectDir}/third_party/zero-allocation-hashing/main/java" - } proto { srcDir 'third_party/cel-spec/src/main/proto' srcDir 'third_party/envoy/src/main/proto' @@ -27,11 +24,6 @@ sourceSets { main { output.classesDirs.from(sourceSets.thirdparty.output.classesDirs) } - test { - java { - srcDir "${projectDir}/third_party/zero-allocation-hashing/test/java" - } - } } configurations { @@ -50,6 +42,7 @@ dependencies { project(':grpc-util'), project(':grpc-services'), project(':grpc-auth'), + project(':grpc-third-party:zero-allocation-hashing'), project(path: ':grpc-alts', configuration: 'shadow'), libraries.guava, libraries.gson, diff --git a/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java b/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java index 21ee914ff8f..848656610e4 100644 --- a/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java @@ -39,6 +39,7 @@ import io.grpc.Metadata; import io.grpc.Status; import io.grpc.SynchronizationContext; +import io.grpc.tp.zah.XxHash64; import io.grpc.util.MultiChildLoadBalancer; import io.grpc.xds.ThreadSafeRandom.ThreadSafeRandomImpl; import io.grpc.xds.client.XdsLogger; diff --git a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java index 58d1ff769fe..44ab55700b1 100644 --- a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java +++ b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java @@ -49,6 +49,7 @@ import io.grpc.SynchronizationContext; import io.grpc.internal.GrpcUtil; import io.grpc.internal.ObjectPool; +import io.grpc.tp.zah.XxHash64; import io.grpc.xds.ClusterSpecifierPlugin.PluginConfig; import io.grpc.xds.Filter.FilterConfig; import io.grpc.xds.Filter.NamedFilterConfig; diff --git a/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java index d65cf96c00d..3c883414b5a 100644 --- a/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java @@ -66,6 +66,7 @@ import io.grpc.internal.PickFirstLoadBalancerProviderAccessor; import io.grpc.internal.PickSubchannelArgsImpl; import io.grpc.testing.TestMethodDescriptors; +import io.grpc.tp.zah.XxHash64; import io.grpc.util.AbstractTestHelper; import io.grpc.util.ForwardingLoadBalancerHelper; import io.grpc.util.MultiChildLoadBalancer.ChildLbState; From 8ec136ae877d05413c4c621231a19f38a9b74c55 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Wed, 24 Sep 2025 11:23:31 +0000 Subject: [PATCH 02/19] Implemented random_subsetting LB policy --- .../io/grpc/LoadBalancerRegistryTest.java | 7 +- util/BUILD.bazel | 1 + util/build.gradle | 1 + .../util/RandomSubsettingLoadBalancer.java | 158 +++++++++ .../RandomSubsettingLoadBalancerProvider.java | 86 +++++ .../services/io.grpc.LoadBalancerProvider | 1 + ...domSubsettingLoadBalancerProviderTest.java | 133 +++++++ .../RandomSubsettingLoadBalancerTest.java | 326 ++++++++++++++++++ 8 files changed, 712 insertions(+), 1 deletion(-) create mode 100644 util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java create mode 100644 util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancerProvider.java create mode 100644 util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerProviderTest.java create mode 100644 util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java diff --git a/api/src/test/java/io/grpc/LoadBalancerRegistryTest.java b/api/src/test/java/io/grpc/LoadBalancerRegistryTest.java index 5b348b7adab..a30d26a78f6 100644 --- a/api/src/test/java/io/grpc/LoadBalancerRegistryTest.java +++ b/api/src/test/java/io/grpc/LoadBalancerRegistryTest.java @@ -40,7 +40,7 @@ public void getClassesViaHardcoded_classesPresent() throws Exception { @Test public void stockProviders() { LoadBalancerRegistry defaultRegistry = LoadBalancerRegistry.getDefaultRegistry(); - assertThat(defaultRegistry.providers()).hasSize(3); + assertThat(defaultRegistry.providers()).hasSize(4); LoadBalancerProvider pickFirst = defaultRegistry.getProvider("pick_first"); assertThat(pickFirst).isInstanceOf(PickFirstLoadBalancerProvider.class); @@ -56,6 +56,11 @@ public void stockProviders() { assertThat(outlierDetection.getClass().getName()).isEqualTo( "io.grpc.util.OutlierDetectionLoadBalancerProvider"); assertThat(roundRobin.getPriority()).isEqualTo(5); + + LoadBalancerProvider randomSubsetting = defaultRegistry.getProvider("random_subsetting"); + assertThat(randomSubsetting.getClass().getName()).isEqualTo( + "io.grpc.util.RandomSubsettingLoadBalancerProvider"); + assertThat(randomSubsetting.getPriority()).isEqualTo(5); } @Test diff --git a/util/BUILD.bazel b/util/BUILD.bazel index 32d5a367b95..ef22c733511 100644 --- a/util/BUILD.bazel +++ b/util/BUILD.bazel @@ -13,6 +13,7 @@ java_library( deps = [ "//api", "//core:internal", + "//third-party/zero-allocation-hashing", artifact("com.google.code.findbugs:jsr305"), artifact("com.google.errorprone:error_prone_annotations"), artifact("com.google.guava:guava"), diff --git a/util/build.gradle b/util/build.gradle index 6fbd6925c00..17ed12daa59 100644 --- a/util/build.gradle +++ b/util/build.gradle @@ -19,6 +19,7 @@ dependencies { api project(':grpc-api') implementation project(':grpc-core'), + project(':grpc-third-party:zero-allocation-hashing'), libraries.animalsniffer.annotations, libraries.guava testImplementation libraries.guava.testlib, diff --git a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java new file mode 100644 index 00000000000..ec32f5058c1 --- /dev/null +++ b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java @@ -0,0 +1,158 @@ +/* + * Copyright 2025 The 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. + */ + +package io.grpc.util; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; + +import io.grpc.EquivalentAddressGroup; +import io.grpc.Internal; +import io.grpc.LoadBalancer; +import io.grpc.Status; +import io.grpc.tp.zah.XxHash64; +import java.security.SecureRandom; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; + + +/** + * Wraps a child {@code LoadBalancer}, separating the total set of backends into smaller subsets for + * the child balancer to balance across. + * + *

This implements random subsetting gRFC: + * https://https://github.com/grpc/proposal/blob/master/A68-random-subsetting.md + */ +@Internal +public final class RandomSubsettingLoadBalancer extends LoadBalancer { + private final GracefulSwitchLoadBalancer switchLb; + + public RandomSubsettingLoadBalancer(Helper helper) { + switchLb = new GracefulSwitchLoadBalancer(checkNotNull(helper, "helper")); + } + + @Override + public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { + RandomSubsettingLoadBalancerConfig config = + (RandomSubsettingLoadBalancerConfig) + resolvedAddresses.getLoadBalancingPolicyConfig(); + + ResolvedAddresses subsetAddresses = filterEndpoints( + resolvedAddresses, config.subsetSize, new SecureRandom().nextLong()); + + return switchLb.acceptResolvedAddresses( + subsetAddresses.toBuilder() + .setLoadBalancingPolicyConfig(config.childConfig) + .build()); + } + + // implements the subsetting algorithm, as described in A68: + // https://github.com/grpc/proposal/pull/423 + private ResolvedAddresses filterEndpoints( + ResolvedAddresses resolvedAddresses, long subsetSize, long seed) { + // configured subset sizes in the range [Integer.MAX_VALUE, Long.MAX_VALUE] will always fall + // into this if statement due to collection indexing limitations in JVM + if (subsetSize >= resolvedAddresses.getAddresses().size()) { + return resolvedAddresses; + } + + XxHash64 hashFunc = new XxHash64(seed); + ArrayList endpointWithHashList = new ArrayList<>(); + + for (EquivalentAddressGroup addressGroup : resolvedAddresses.getAddresses()) { + endpointWithHashList.add( + new EndpointWithHash( + addressGroup, + hashFunc.hashAsciiString(addressGroup.getAddresses().get(0).toString()))); + } + + Collections.sort(endpointWithHashList, new HashAddressComparator()); + + ArrayList addressGroups = new ArrayList<>(); + + // for loop is executed for subset sizes in range [0, Integer.MAX_VALUE), therefore indexing + // variable is not going to overflow here + for (int idx = 0; idx < subsetSize; ++idx) { + addressGroups.add(endpointWithHashList.get(idx).addressGroup); + } + + return resolvedAddresses.toBuilder().setAddresses(addressGroups).build(); + } + + @Override + public void handleNameResolutionError(Status error) { + switchLb.handleNameResolutionError(error); + } + + @Override + public void shutdown() { + switchLb.shutdown(); + } + + private static final class EndpointWithHash { + public final EquivalentAddressGroup addressGroup; + public final long hash; + + public EndpointWithHash(EquivalentAddressGroup addressGroup, long hash) { + this.addressGroup = addressGroup; + this.hash = hash; + } + } + + private static final class HashAddressComparator implements Comparator { + @Override + public int compare(EndpointWithHash lhs, EndpointWithHash rhs) { + return Long.compare(lhs.hash, rhs.hash); + } + } + + public static final class RandomSubsettingLoadBalancerConfig { + public final long subsetSize; + public final Object childConfig; + + private RandomSubsettingLoadBalancerConfig(long subsetSize, Object childConfig) { + this.subsetSize = subsetSize; + this.childConfig = childConfig; + } + + public static class Builder { + Long subsetSize; + Object childConfig; + + public Builder setSubsetSize(Integer subsetSize) { + checkNotNull(subsetSize, "subsetSize"); + // {@code Integer.toUnsignedLong(int)} is not part of Android API level 21, therefore doing + // it manually + Long subsetSizeAsLong = ((long) subsetSize) & 0xFFFFFFFFL; + checkArgument(subsetSizeAsLong > 0L, "Subset size must be greater than 0"); + this.subsetSize = subsetSizeAsLong; + return this; + } + + public Builder setChildConfig(Object childConfig) { + this.childConfig = checkNotNull(childConfig, "childConfig"); + return this; + } + + public RandomSubsettingLoadBalancerConfig build() { + return new RandomSubsettingLoadBalancerConfig( + checkNotNull(subsetSize, "subsetSize"), + checkNotNull(childConfig, "childConfig")); + } + } + } +} diff --git a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancerProvider.java b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancerProvider.java new file mode 100644 index 00000000000..b8c15b663cc --- /dev/null +++ b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancerProvider.java @@ -0,0 +1,86 @@ +/* + * Copyright 2025 The 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. + */ + +package io.grpc.util; + +import io.grpc.Internal; +import io.grpc.LoadBalancer; +import io.grpc.LoadBalancerProvider; +import io.grpc.NameResolver.ConfigOrError; +import io.grpc.Status; +import io.grpc.internal.JsonUtil; +import java.util.Map; + +@Internal +public final class RandomSubsettingLoadBalancerProvider extends LoadBalancerProvider { + private static final String POLICY_NAME = "random_subsetting"; + + @Override + public LoadBalancer newLoadBalancer(LoadBalancer.Helper helper) { + return new RandomSubsettingLoadBalancer(helper); + } + + @Override + public boolean isAvailable() { + return true; + } + + @Override + public int getPriority() { + return 5; + } + + @Override + public String getPolicyName() { + return POLICY_NAME; + } + + @Override + public ConfigOrError parseLoadBalancingPolicyConfig(Map rawConfig) { + try { + return parseLoadBalancingPolicyConfigInternal(rawConfig); + } catch (RuntimeException e) { + return ConfigOrError.fromError( + Status.UNAVAILABLE + .withCause(e) + .withDescription("Failed parsing configuration for " + getPolicyName())); + } + } + + private ConfigOrError parseLoadBalancingPolicyConfigInternal(Map rawConfig) { + Integer subsetSize = JsonUtil.getNumberAsInteger(rawConfig, "subsetSize"); + if (subsetSize == null) { + return ConfigOrError.fromError( + Status.INTERNAL.withDescription( + "Subset size missing in " + getPolicyName() + ", LB policy config=" + rawConfig)); + } + + ConfigOrError childConfig = GracefulSwitchLoadBalancer.parseLoadBalancingPolicyConfig( + JsonUtil.getListOfObjects(rawConfig, "childPolicy")); + if (childConfig.getError() != null) { + return ConfigOrError.fromError(Status.INTERNAL + .withDescription( + "Failed to parse child in " + getPolicyName() + ", LB policy config=" + rawConfig) + .withCause(childConfig.getError().asRuntimeException())); + } + + return ConfigOrError.fromConfig( + new RandomSubsettingLoadBalancer.RandomSubsettingLoadBalancerConfig.Builder() + .setSubsetSize(subsetSize) + .setChildConfig(childConfig.getConfig()) + .build()); + } +} diff --git a/util/src/main/resources/META-INF/services/io.grpc.LoadBalancerProvider b/util/src/main/resources/META-INF/services/io.grpc.LoadBalancerProvider index 1fdd69cb00b..d973a6f6728 100644 --- a/util/src/main/resources/META-INF/services/io.grpc.LoadBalancerProvider +++ b/util/src/main/resources/META-INF/services/io.grpc.LoadBalancerProvider @@ -1,2 +1,3 @@ io.grpc.util.SecretRoundRobinLoadBalancerProvider$Provider io.grpc.util.OutlierDetectionLoadBalancerProvider +io.grpc.util.RandomSubsettingLoadBalancerProvider diff --git a/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerProviderTest.java b/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerProviderTest.java new file mode 100644 index 00000000000..830ad9723d8 --- /dev/null +++ b/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerProviderTest.java @@ -0,0 +1,133 @@ +/* + * Copyright 2025 The 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. + */ + +package io.grpc.util; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; + +import io.grpc.InternalServiceProviders; +import io.grpc.LoadBalancer.Helper; +import io.grpc.LoadBalancerProvider; +import io.grpc.NameResolver.ConfigOrError; +import io.grpc.Status; +import io.grpc.internal.JsonParser; +import io.grpc.util.RandomSubsettingLoadBalancer.RandomSubsettingLoadBalancerConfig; +import java.io.IOException; +import java.util.Map; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class RandomSubsettingLoadBalancerProviderTest { + private final RandomSubsettingLoadBalancerProvider provider = + new RandomSubsettingLoadBalancerProvider(); + + @Test + public void registered() { + for (LoadBalancerProvider current : + InternalServiceProviders.getCandidatesViaServiceLoader( + LoadBalancerProvider.class, getClass().getClassLoader())) { + if (current instanceof RandomSubsettingLoadBalancerProvider) { + return; + } + } + fail("RandomSubsettingLoadBalancerProvider not registered"); + } + + @Test + public void providesLoadBalancer() { + Helper helper = mock(Helper.class); + assertThat(provider.newLoadBalancer(helper)) + .isInstanceOf(RandomSubsettingLoadBalancer.class); + } + + @Test + public void parseConfigRequiresSubsetSize() throws IOException { + String emptyConfig = "{}"; + + ConfigOrError configOrError = + provider.parseLoadBalancingPolicyConfig(parseJsonObject(emptyConfig)); + assertThat(configOrError.getError()).isNotNull(); + assertThat(configOrError.getError().toString()) + .isEqualTo( + Status.INTERNAL + .withDescription("Subset size missing in random_subsetting, LB policy config={}") + .toString()); + } + + @Test + public void parseConfigReturnsErrorWhenChildPolicyMissing() throws IOException { + String missingChildPolicyConfig = "{\"subsetSize\": 3}"; + + ConfigOrError configOrError = + provider.parseLoadBalancingPolicyConfig(parseJsonObject(missingChildPolicyConfig)); + assertThat(configOrError.getError()).isNotNull(); + + Status error = configOrError.getError(); + assertThat(error.getCode()).isEqualTo(Status.Code.INTERNAL); + assertThat(error.getDescription()).isEqualTo( + "Failed to parse child in random_subsetting" + + ", LB policy config={subsetSize=3.0}"); + assertThat(error.getCause().getMessage()).isEqualTo("INTERNAL: No child LB config specified"); + } + + @Test + public void parseConfigReturnsErrorWhenChildPolicyInvalid() throws IOException { + String invalidChildPolicyConfig = + "{" + + "\"subsetSize\": 3, " + + "\"childPolicy\" : [{\"random_policy\" : {}}]" + + "}"; + + ConfigOrError configOrError = + provider.parseLoadBalancingPolicyConfig(parseJsonObject(invalidChildPolicyConfig)); + assertThat(configOrError.getError()).isNotNull(); + + Status error = configOrError.getError(); + assertThat(error.getCode()).isEqualTo(Status.Code.INTERNAL); + assertThat(error.getDescription()).isEqualTo( + "Failed to parse child in random_subsetting, LB policy config=" + + "{subsetSize=3.0, childPolicy=[{random_policy={}}]}"); + assertThat(error.getCause().getMessage()).contains( + "INTERNAL: None of [random_policy] specified by Service Config are available."); + } + + @Test + public void parseValidConfig() throws IOException { + String validConfig = + "{" + + "\"subsetSize\": 3, " + + "\"childPolicy\" : [{\"round_robin\" : {}}]" + + "}"; + ConfigOrError configOrError = + provider.parseLoadBalancingPolicyConfig(parseJsonObject(validConfig)); + assertThat(configOrError.getConfig()).isNotNull(); + + RandomSubsettingLoadBalancerConfig actualConfig = + (RandomSubsettingLoadBalancerConfig) configOrError.getConfig(); + assertThat(GracefulSwitchLoadBalancerAccessor.getChildProvider( + actualConfig.childConfig).getPolicyName()).isEqualTo("round_robin"); + assertThat(actualConfig.subsetSize).isEqualTo(3); + } + + @SuppressWarnings("unchecked") + private static Map parseJsonObject(String json) throws IOException { + return (Map) JsonParser.parse(json); + } +} diff --git a/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java b/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java new file mode 100644 index 00000000000..a16a4d98558 --- /dev/null +++ b/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java @@ -0,0 +1,326 @@ +/* + * Copyright 2025 The 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. + */ + +package io.grpc.util; + +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import io.grpc.ConnectivityState; +import io.grpc.ConnectivityStateInfo; +import io.grpc.EquivalentAddressGroup; +import io.grpc.LoadBalancer; +import io.grpc.LoadBalancer.CreateSubchannelArgs; +import io.grpc.LoadBalancer.ResolvedAddresses; +import io.grpc.LoadBalancer.Subchannel; +import io.grpc.LoadBalancer.SubchannelStateListener; +import io.grpc.LoadBalancerProvider; +import io.grpc.Status; +import io.grpc.internal.TestUtils; +import io.grpc.util.RandomSubsettingLoadBalancer.RandomSubsettingLoadBalancerConfig; +import java.net.SocketAddress; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; +import org.mockito.stubbing.Answer; + +public class RandomSubsettingLoadBalancerTest { + @Rule + public final MockitoRule mockitoRule = MockitoJUnit.rule(); + + @Mock + private LoadBalancer.Helper mockHelper; + @Mock + private LoadBalancer mockChildLb; + @Mock + private SocketAddress mockSocketAddress; + + @Captor + private ArgumentCaptor resolvedAddrCaptor; + + private BackendDetails backendDetails; + + private RandomSubsettingLoadBalancer loadBalancer; + + private final LoadBalancerProvider mockChildLbProvider = + new TestUtils.StandardLoadBalancerProvider("foo_policy") { + @Override + public LoadBalancer newLoadBalancer(LoadBalancer.Helper helper) { + return mockChildLb; + } + }; + + private final LoadBalancerProvider roundRobinLbProvider = + new TestUtils.StandardLoadBalancerProvider("round_robin") { + @Override + public LoadBalancer newLoadBalancer(LoadBalancer.Helper helper) { + return new RoundRobinLoadBalancer(helper); + } + }; + + private Object newChildConfig(LoadBalancerProvider provider, Object config) { + return GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig(provider, config); + } + + private RandomSubsettingLoadBalancerConfig createRandomSubsettingLbConfig( + int subsetSize, LoadBalancerProvider childLbProvider, Object childConfig) { + return new RandomSubsettingLoadBalancer.RandomSubsettingLoadBalancerConfig.Builder() + .setSubsetSize(subsetSize) + .setChildConfig(newChildConfig(childLbProvider, childConfig)) + .build(); + } + + private BackendDetails setupBackends(int backendCount) { + List servers = Lists.newArrayList(); + Map, Subchannel> subchannels = Maps.newLinkedHashMap(); + + for (int i = 0; i < backendCount; i++) { + SocketAddress addr = new FakeSocketAddress("server" + i); + EquivalentAddressGroup addressGroup = new EquivalentAddressGroup(addr); + servers.add(addressGroup); + Subchannel subchannel = mock(Subchannel.class); + subchannels.put(Arrays.asList(addressGroup), subchannel); + } + + return new BackendDetails(servers, subchannels); + } + + @Before + public void setUp() { + loadBalancer = new RandomSubsettingLoadBalancer(mockHelper); + + int backendSize = 5; + backendDetails = setupBackends(backendSize); + } + + @Test + public void handleNameResolutionError() { + int subsetSize = 2; + Object childConfig = "someConfig"; + + RandomSubsettingLoadBalancerConfig config = createRandomSubsettingLbConfig( + subsetSize, mockChildLbProvider, childConfig); + + loadBalancer.acceptResolvedAddresses( + ResolvedAddresses.newBuilder() + .setAddresses(ImmutableList.of(new EquivalentAddressGroup(mockSocketAddress))) + .setLoadBalancingPolicyConfig(config) + .build()); + + loadBalancer.handleNameResolutionError(Status.DEADLINE_EXCEEDED); + verify(mockChildLb).handleNameResolutionError(Status.DEADLINE_EXCEEDED); + } + + @Test + public void shutdown() { + int subsetSize = 2; + Object childConfig = "someConfig"; + + RandomSubsettingLoadBalancerConfig config = createRandomSubsettingLbConfig( + subsetSize, mockChildLbProvider, childConfig); + + loadBalancer.acceptResolvedAddresses( + ResolvedAddresses.newBuilder() + .setAddresses(ImmutableList.of(new EquivalentAddressGroup(mockSocketAddress))) + .setLoadBalancingPolicyConfig(config) + .build()); + + loadBalancer.shutdown(); + verify(mockChildLb).shutdown(); + } + + @Test + public void acceptResolvedAddresses_mockedChildLbPolicy() { + int subsetSize = 3; + Object childConfig = "someConfig"; + + RandomSubsettingLoadBalancerConfig config = createRandomSubsettingLbConfig( + subsetSize, mockChildLbProvider, childConfig); + + ResolvedAddresses resolvedAddresses = + ResolvedAddresses.newBuilder() + .setAddresses(ImmutableList.copyOf(backendDetails.servers)) + .setLoadBalancingPolicyConfig(config) + .build(); + + loadBalancer.acceptResolvedAddresses(resolvedAddresses); + + verify(mockChildLb).acceptResolvedAddresses(resolvedAddrCaptor.capture()); + assertThat(resolvedAddrCaptor.getValue().getAddresses().size()).isEqualTo(subsetSize); + assertThat(resolvedAddrCaptor.getValue().getLoadBalancingPolicyConfig()).isEqualTo(childConfig); + } + + @Test + public void acceptResolvedAddresses_roundRobinChildLbPolicy() { + int subsetSize = 3; + Object childConfig = null; + + RandomSubsettingLoadBalancerConfig config = createRandomSubsettingLbConfig( + subsetSize, roundRobinLbProvider, childConfig); + + ResolvedAddresses resolvedAddresses = + ResolvedAddresses.newBuilder() + .setAddresses(ImmutableList.copyOf(backendDetails.servers)) + .setLoadBalancingPolicyConfig(config) + .build(); + + loadBalancer.acceptResolvedAddresses(resolvedAddresses); + + int insubset = 0; + for (Subchannel subchannel : backendDetails.subchannels.values()) { + LoadBalancer.SubchannelStateListener ssl = + backendDetails.subchannelStateListeners.get(subchannel); + if (ssl != null) { // it might be null if it's not in the subset. + insubset += 1; + ssl.onSubchannelState(ConnectivityStateInfo.forNonError(ConnectivityState.READY)); + } + } + + assertThat(insubset).isEqualTo(subsetSize); + } + + // verifies: https://github.com/grpc/proposal/blob/master/A68_graphics/subsetting100-100-5.png + @Test + public void backendsCanBeDistributedEvenly_subsetting100_100_5() { + verifyConnectionsByServer(100, 100, 5, 15); + } + + // verifies https://github.com/grpc/proposal/blob/master/A68_graphics/subsetting100-100-25.png + @Test + public void backendsCanBeDistributedEvenly_subsetting100_100_25() { + verifyConnectionsByServer(100, 100, 25, 40); + } + + // verifies: https://github.com/grpc/proposal/blob/master/A68_graphics/subsetting100-10-5.png + @Test + public void backendsCanBeDistributedEvenly_subsetting100_10_5() { + verifyConnectionsByServer(100, 10, 5, 65); + } + + // verifies: https://github.com/grpc/proposal/blob/master/A68_graphics/subsetting500-10-5.png + @Test + public void backendsCanBeDistributedEvenly_subsetting500_10_5() { + verifyConnectionsByServer(500, 10, 5, 600); + } + + // verifies: https://github.com/grpc/proposal/blob/master/A68_graphics/subsetting2000-10-5.png + @Test + public void backendsCanBeDistributedEvenly_subsetting2000_100_5() { + verifyConnectionsByServer(2000, 10, 5, 1200); + } + + public void verifyConnectionsByServer( + int clientsCount, int serversCount, int subsetSize, int expectedMaxConnections) { + backendDetails = setupBackends(serversCount); + Object childConfig = "someConfig"; + + List configs = Lists.newArrayList(); + for (int i = 0; i < clientsCount; i++) { + configs.add(createRandomSubsettingLbConfig(subsetSize, mockChildLbProvider, childConfig)); + } + + Map connectionsByServer = Maps.newLinkedHashMap(); + + for (RandomSubsettingLoadBalancerConfig config : configs) { + ResolvedAddresses resolvedAddresses = + ResolvedAddresses.newBuilder() + .setAddresses(ImmutableList.copyOf(backendDetails.servers)) + .setLoadBalancingPolicyConfig(config) + .build(); + + loadBalancer.acceptResolvedAddresses(resolvedAddresses); + + verify(mockChildLb, atLeastOnce()).acceptResolvedAddresses(resolvedAddrCaptor.capture()); + // Verify ChildLB is only getting subsetSize ResolvedAddresses each time + assertThat(resolvedAddrCaptor.getValue().getAddresses().size()).isEqualTo(config.subsetSize); + + for (EquivalentAddressGroup eag : resolvedAddrCaptor.getValue().getAddresses()) { + for (SocketAddress addr : eag.getAddresses()) { + Integer prev = connectionsByServer.getOrDefault(addr, 0); + connectionsByServer.put(addr, prev + 1); + } + } + } + + int maxConnections = Collections.max(connectionsByServer.values()); + + assertThat(maxConnections <= expectedMaxConnections).isTrue(); + } + + private class BackendDetails { + private final List servers; + private final Map, Subchannel> subchannels; + private final Map subchannelStateListeners; + + BackendDetails(List servers, + Map, Subchannel> subchannels) { + this.servers = servers; + this.subchannels = subchannels; + this.subchannelStateListeners = Maps.newLinkedHashMap(); + + when(mockHelper.createSubchannel(any(LoadBalancer.CreateSubchannelArgs.class))).then( + new Answer() { + @Override + public Subchannel answer(InvocationOnMock invocation) throws Throwable { + CreateSubchannelArgs args = (CreateSubchannelArgs) invocation.getArguments()[0]; + final Subchannel subchannel = backendDetails.subchannels.get(args.getAddresses()); + when(subchannel.getAllAddresses()).thenReturn(args.getAddresses()); + when(subchannel.getAttributes()).thenReturn(args.getAttributes()); + doAnswer(new Answer() { + @Override + public Void answer(InvocationOnMock invocation) throws Throwable { + subchannelStateListeners.put(subchannel, + (SubchannelStateListener) invocation.getArguments()[0]); + return null; + } + }).when(subchannel).start(any(SubchannelStateListener.class)); + return subchannel; + } + }); + } + } + + private static class FakeSocketAddress extends SocketAddress { + final String name; + + FakeSocketAddress(String name) { + this.name = name; + } + + @Override + public String toString() { + return "FakeSocketAddress-" + name; + } + } +} From 1870e6fdade89b7f88b30925fc97708ab4cc788f Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Wed, 24 Sep 2025 11:25:47 +0000 Subject: [PATCH 03/19] Implemented random_subsetting converter for xDS --- .../grpc/xds/LoadBalancerConfigFactory.java | 29 ++++++++++++++++ .../v3/random_subsetting.proto | 33 +++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 xds/third_party/envoy/src/main/proto/envoy/extensions/load_balancing_policies/random_subsetting/v3/random_subsetting.proto diff --git a/xds/src/main/java/io/grpc/xds/LoadBalancerConfigFactory.java b/xds/src/main/java/io/grpc/xds/LoadBalancerConfigFactory.java index e08ea0fab43..c934eb5843e 100644 --- a/xds/src/main/java/io/grpc/xds/LoadBalancerConfigFactory.java +++ b/xds/src/main/java/io/grpc/xds/LoadBalancerConfigFactory.java @@ -32,6 +32,7 @@ import io.envoyproxy.envoy.extensions.load_balancing_policies.client_side_weighted_round_robin.v3.ClientSideWeightedRoundRobin; import io.envoyproxy.envoy.extensions.load_balancing_policies.least_request.v3.LeastRequest; import io.envoyproxy.envoy.extensions.load_balancing_policies.pick_first.v3.PickFirst; +import io.envoyproxy.envoy.extensions.load_balancing_policies.random_subsetting.v3.RandomSubsetting; import io.envoyproxy.envoy.extensions.load_balancing_policies.ring_hash.v3.RingHash; import io.envoyproxy.envoy.extensions.load_balancing_policies.round_robin.v3.RoundRobin; import io.envoyproxy.envoy.extensions.load_balancing_policies.wrr_locality.v3.WrrLocality; @@ -92,6 +93,9 @@ class LoadBalancerConfigFactory { static final String ERROR_UTILIZATION_PENALTY = "errorUtilizationPenalty"; + static final String RANDOM_SUBSETTING_FIELD_NAME = "random_subsetting"; + static final String SUBSET_SIZE = "subsetSize"; + /** * Factory method for creating a new {link LoadBalancerConfigConverter} for a given xDS {@link * Cluster}. @@ -200,6 +204,20 @@ class LoadBalancerConfigFactory { return ImmutableMap.of(PICK_FIRST_FIELD_NAME, configBuilder.buildOrThrow()); } + /** + * Builds a service config JSON object for the random_subsetting load balancer config based on the + * given config values. + */ + private static ImmutableMap buildRandomSubsettingConfig( + RandomSubsetting randomSubsetting) { + return ImmutableMap.of( + RANDOM_SUBSETTING_FIELD_NAME, + ImmutableMap.of( + SUBSET_SIZE, randomSubsetting.getSubsetSize(), + CHILD_POLICY_FIELD, randomSubsetting.getChildPolicy() + )); + } + /** * Responsible for converting from a {@code envoy.config.cluster.v3.LoadBalancingPolicy} proto * message to a gRPC service config format. @@ -236,6 +254,9 @@ static class LoadBalancingPolicyConverter { typedConfig.unpack(ClientSideWeightedRoundRobin.class)); } else if (typedConfig.is(PickFirst.class)) { serviceConfig = convertPickFirstConfig(typedConfig.unpack(PickFirst.class)); + } else if (typedConfig.is(RandomSubsetting.class)) { + serviceConfig = convertRandomSubsettingConfig( + typedConfig.unpack(RandomSubsetting.class)); } else if (typedConfig.is(com.github.xds.type.v3.TypedStruct.class)) { serviceConfig = convertCustomConfig( typedConfig.unpack(com.github.xds.type.v3.TypedStruct.class)); @@ -324,6 +345,14 @@ static class LoadBalancingPolicyConverter { return buildPickFirstConfig(pickFirst.getShuffleAddressList()); } + /** + * "Converts" a random_subsetting configuration to service config format. + */ + private static ImmutableMap convertRandomSubsettingConfig( + RandomSubsetting randomSubsetting) { + return buildRandomSubsettingConfig(randomSubsetting); + } + /** * Converts a least_request {@link Any} configuration to service config format. */ diff --git a/xds/third_party/envoy/src/main/proto/envoy/extensions/load_balancing_policies/random_subsetting/v3/random_subsetting.proto b/xds/third_party/envoy/src/main/proto/envoy/extensions/load_balancing_policies/random_subsetting/v3/random_subsetting.proto new file mode 100644 index 00000000000..0690397f770 --- /dev/null +++ b/xds/third_party/envoy/src/main/proto/envoy/extensions/load_balancing_policies/random_subsetting/v3/random_subsetting.proto @@ -0,0 +1,33 @@ +syntax = "proto3"; + +package envoy.extensions.load_balancing_policies.random_subsetting.v3; + +import "envoy/config/cluster/v3/cluster.proto"; + +import "google/protobuf/wrappers.proto"; + +import "udpa/annotations/status.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.load_balancing_policies.random_subsetting.v3"; +option java_outer_classname = "RandomSubsettingProto"; +option java_multiple_files = true; +option go_package = "github.com/envoyproxy/go-control-plane/envoy/extensions/load_balancing_policies/random_subsetting/v3;random_subsettingv3"; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Random Subsetting Load Balancing Policy] +// [#extension: envoy.load_balancing_policies.random_subsetting] + +message RandomSubsetting { + // subset_size indicates how many backends every client will be connected to. + // The value must be greater than 0. + google.protobuf.UInt32Value subset_size = 1 [ + (validate.rules).uint32 = {gt: 0} + ]; + + // The config for the child policy. + // The value is required. + config.cluster.v3.LoadBalancingPolicy child_policy = 2 [ + (validate.rules).message = {required: true} + ]; +} From 40af1a8614a9988ff942843e08c9372c447470b6 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Mon, 6 Oct 2025 12:26:50 +0000 Subject: [PATCH 04/19] Optimized seed lifetime --- .../io/grpc/util/RandomSubsettingLoadBalancer.java | 12 ++++++------ .../grpc/util/RandomSubsettingLoadBalancerTest.java | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java index ec32f5058c1..a729659335b 100644 --- a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java @@ -24,10 +24,10 @@ import io.grpc.LoadBalancer; import io.grpc.Status; import io.grpc.tp.zah.XxHash64; -import java.security.SecureRandom; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.Random; /** @@ -40,9 +40,12 @@ @Internal public final class RandomSubsettingLoadBalancer extends LoadBalancer { private final GracefulSwitchLoadBalancer switchLb; + private final XxHash64 hashFunc; public RandomSubsettingLoadBalancer(Helper helper) { switchLb = new GracefulSwitchLoadBalancer(checkNotNull(helper, "helper")); + long seed = new Random().nextLong(); + hashFunc = new XxHash64(seed); } @Override @@ -51,8 +54,7 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { (RandomSubsettingLoadBalancerConfig) resolvedAddresses.getLoadBalancingPolicyConfig(); - ResolvedAddresses subsetAddresses = filterEndpoints( - resolvedAddresses, config.subsetSize, new SecureRandom().nextLong()); + ResolvedAddresses subsetAddresses = filterEndpoints(resolvedAddresses, config.subsetSize); return switchLb.acceptResolvedAddresses( subsetAddresses.toBuilder() @@ -62,15 +64,13 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { // implements the subsetting algorithm, as described in A68: // https://github.com/grpc/proposal/pull/423 - private ResolvedAddresses filterEndpoints( - ResolvedAddresses resolvedAddresses, long subsetSize, long seed) { + private ResolvedAddresses filterEndpoints(ResolvedAddresses resolvedAddresses, long subsetSize) { // configured subset sizes in the range [Integer.MAX_VALUE, Long.MAX_VALUE] will always fall // into this if statement due to collection indexing limitations in JVM if (subsetSize >= resolvedAddresses.getAddresses().size()) { return resolvedAddresses; } - XxHash64 hashFunc = new XxHash64(seed); ArrayList endpointWithHashList = new ArrayList<>(); for (EquivalentAddressGroup addressGroup : resolvedAddresses.getAddresses()) { diff --git a/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java b/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java index a16a4d98558..245f8f6bedd 100644 --- a/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java +++ b/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java @@ -259,6 +259,7 @@ public void verifyConnectionsByServer( .setLoadBalancingPolicyConfig(config) .build(); + loadBalancer = new RandomSubsettingLoadBalancer(mockHelper); loadBalancer.acceptResolvedAddresses(resolvedAddresses); verify(mockChildLb, atLeastOnce()).acceptResolvedAddresses(resolvedAddrCaptor.capture()); From d78cf202a90c11a5569e3e072ffa4fd12d19fdce Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Mon, 6 Oct 2025 12:38:25 +0000 Subject: [PATCH 05/19] Optimized memory allocations for arrays --- .../java/io/grpc/util/RandomSubsettingLoadBalancer.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java index a729659335b..b2a975b29b4 100644 --- a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java @@ -71,7 +71,8 @@ private ResolvedAddresses filterEndpoints(ResolvedAddresses resolvedAddresses, l return resolvedAddresses; } - ArrayList endpointWithHashList = new ArrayList<>(); + ArrayList endpointWithHashList = + new ArrayList<>(resolvedAddresses.getAddresses().size()); for (EquivalentAddressGroup addressGroup : resolvedAddresses.getAddresses()) { endpointWithHashList.add( @@ -82,7 +83,9 @@ private ResolvedAddresses filterEndpoints(ResolvedAddresses resolvedAddresses, l Collections.sort(endpointWithHashList, new HashAddressComparator()); - ArrayList addressGroups = new ArrayList<>(); + // array is constructed for subset sizes in range [0, Integer.MAX_VALUE), therefore casting + // from long to int is not going to overflow here + ArrayList addressGroups = new ArrayList<>((int) subsetSize); // for loop is executed for subset sizes in range [0, Integer.MAX_VALUE), therefore indexing // variable is not going to overflow here From 4dfa2e77141a7bbfd044d0828dcc4d3e39ea1663 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Mon, 6 Oct 2025 12:48:38 +0000 Subject: [PATCH 06/19] Changed visibility of LB to package-private --- .../main/java/io/grpc/util/RandomSubsettingLoadBalancer.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java index b2a975b29b4..fabf193f3af 100644 --- a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java @@ -20,7 +20,6 @@ import static com.google.common.base.Preconditions.checkNotNull; import io.grpc.EquivalentAddressGroup; -import io.grpc.Internal; import io.grpc.LoadBalancer; import io.grpc.Status; import io.grpc.tp.zah.XxHash64; @@ -37,8 +36,7 @@ *

This implements random subsetting gRFC: * https://https://github.com/grpc/proposal/blob/master/A68-random-subsetting.md */ -@Internal -public final class RandomSubsettingLoadBalancer extends LoadBalancer { +final class RandomSubsettingLoadBalancer extends LoadBalancer { private final GracefulSwitchLoadBalancer switchLb; private final XxHash64 hashFunc; From f5147eed731c6af8c0a1535be4442ebffd32adb5 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Mon, 6 Oct 2025 12:49:36 +0000 Subject: [PATCH 07/19] Revert "Moved `XxHash64` library to enable shared use across projects" This reverts commit 8d349df5d3108b4e13f35a7508ca7b94c331b8e4. --- settings.gradle | 2 -- .../zero-allocation-hashing/BUILD.bazel | 26 ------------------- .../zero-allocation-hashing/build.gradle | 25 ------------------ xds/BUILD.bazel | 2 +- xds/build.gradle | 9 ++++++- .../io/grpc/xds/RingHashLoadBalancer.java | 1 - .../java/io/grpc/xds/XdsNameResolver.java | 1 - .../io/grpc/xds/RingHashLoadBalancerTest.java | 1 - .../zero-allocation-hashing/LICENSE | 0 .../zero-allocation-hashing/NOTICE | 0 .../main/java/io/grpc/xds}/XxHash64.java | 24 ++++++++--------- .../test/java/io/grpc/xds}/XxHash64Test.java | 2 +- 12 files changed, 22 insertions(+), 71 deletions(-) delete mode 100644 third-party/zero-allocation-hashing/BUILD.bazel delete mode 100644 third-party/zero-allocation-hashing/build.gradle rename {third-party => xds/third_party}/zero-allocation-hashing/LICENSE (100%) rename {third-party => xds/third_party}/zero-allocation-hashing/NOTICE (100%) rename {third-party/zero-allocation-hashing/src/main/java/io/grpc/tp/zah => xds/third_party/zero-allocation-hashing/main/java/io/grpc/xds}/XxHash64.java (93%) rename {third-party/zero-allocation-hashing/src/test/java/io/grpc/tp/zah => xds/third_party/zero-allocation-hashing/test/java/io/grpc/xds}/XxHash64Test.java (99%) diff --git a/settings.gradle b/settings.gradle index 9e9583bc50b..f4df1105090 100644 --- a/settings.gradle +++ b/settings.gradle @@ -93,7 +93,6 @@ include ":grpc-inprocess" include ":grpc-util" include ":grpc-opentelemetry" include ":grpc-context-override-opentelemetry" -include ":grpc-third-party:zero-allocation-hashing" project(':grpc-api').projectDir = "$rootDir/api" as File project(':grpc-core').projectDir = "$rootDir/core" as File @@ -131,7 +130,6 @@ project(':grpc-inprocess').projectDir = "$rootDir/inprocess" as File project(':grpc-util').projectDir = "$rootDir/util" as File project(':grpc-opentelemetry').projectDir = "$rootDir/opentelemetry" as File project(':grpc-context-override-opentelemetry').projectDir = "$rootDir/contextstorage" as File -project(':grpc-third-party:zero-allocation-hashing').projectDir = "$rootDir/third-party/zero-allocation-hashing" as File if (settings.hasProperty('skipCodegen') && skipCodegen.toBoolean()) { println '*** Skipping the build of codegen and compilation of proto files because skipCodegen=true' diff --git a/third-party/zero-allocation-hashing/BUILD.bazel b/third-party/zero-allocation-hashing/BUILD.bazel deleted file mode 100644 index 442f41416c7..00000000000 --- a/third-party/zero-allocation-hashing/BUILD.bazel +++ /dev/null @@ -1,26 +0,0 @@ -load("@rules_java//java:defs.bzl", "java_binary", "java_library", "java_test") - -java_library( - name = "zero-allocation-hashing", - srcs = [ - "src/main/java/io/grpc/tp/zah/XxHash64.java", - ], - deps = [ - "@maven//:com_google_guava_guava", - ], - visibility = [ - "//xds:__pkg__", - "//util:__pkg__", - ], -) - -java_test( - name = "XxHash64Test", - size = "small", - srcs = ["src/test/java/io/grpc/tp/zah/XxHash64Test.java"], - deps = [ - ":zero-allocation-hashing", - "@maven//:com_google_guava_guava", - "@maven//:junit_junit", - ], -) diff --git a/third-party/zero-allocation-hashing/build.gradle b/third-party/zero-allocation-hashing/build.gradle deleted file mode 100644 index 2930206d709..00000000000 --- a/third-party/zero-allocation-hashing/build.gradle +++ /dev/null @@ -1,25 +0,0 @@ -plugins { - id "java-library" -} - -description = 'gRPC: Zero Allocation Hashing' - -dependencies { - implementation libraries.guava - - testImplementation libraries.junit -} - -tasks.named("jar").configure { - manifest { - attributes('Automatic-Module-Name': 'io.grpc.tp.zah') - } -} - -tasks.named("checkstyleMain").configure { - enabled = false -} - -tasks.named("checkstyleTest").configure { - enabled = false -} diff --git a/xds/BUILD.bazel b/xds/BUILD.bazel index 1bddcc35d82..66c790a654d 100644 --- a/xds/BUILD.bazel +++ b/xds/BUILD.bazel @@ -31,7 +31,6 @@ java_library( "//services:metrics", "//services:metrics_internal", "//stub", - "//third-party/zero-allocation-hashing", "//util", "@com_google_protobuf//:protobuf_java", "@com_google_protobuf//:protobuf_java_util", @@ -74,6 +73,7 @@ java_binary( srcs = glob( [ "src/main/java/**/*.java", + "third_party/zero-allocation-hashing/main/java/**/*.java", ], exclude = ["src/main/java/io/grpc/xds/orca/**"], ), diff --git a/xds/build.gradle b/xds/build.gradle index 6c47a54599d..8394fe12f6b 100644 --- a/xds/build.gradle +++ b/xds/build.gradle @@ -13,6 +13,9 @@ description = "gRPC: XDS plugin" sourceSets { thirdparty { + java { + srcDir "${projectDir}/third_party/zero-allocation-hashing/main/java" + } proto { srcDir 'third_party/cel-spec/src/main/proto' srcDir 'third_party/envoy/src/main/proto' @@ -24,6 +27,11 @@ sourceSets { main { output.classesDirs.from(sourceSets.thirdparty.output.classesDirs) } + test { + java { + srcDir "${projectDir}/third_party/zero-allocation-hashing/test/java" + } + } } configurations { @@ -42,7 +50,6 @@ dependencies { project(':grpc-util'), project(':grpc-services'), project(':grpc-auth'), - project(':grpc-third-party:zero-allocation-hashing'), project(path: ':grpc-alts', configuration: 'shadow'), libraries.guava, libraries.gson, diff --git a/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java b/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java index 848656610e4..21ee914ff8f 100644 --- a/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java @@ -39,7 +39,6 @@ import io.grpc.Metadata; import io.grpc.Status; import io.grpc.SynchronizationContext; -import io.grpc.tp.zah.XxHash64; import io.grpc.util.MultiChildLoadBalancer; import io.grpc.xds.ThreadSafeRandom.ThreadSafeRandomImpl; import io.grpc.xds.client.XdsLogger; diff --git a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java index 44ab55700b1..58d1ff769fe 100644 --- a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java +++ b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java @@ -49,7 +49,6 @@ import io.grpc.SynchronizationContext; import io.grpc.internal.GrpcUtil; import io.grpc.internal.ObjectPool; -import io.grpc.tp.zah.XxHash64; import io.grpc.xds.ClusterSpecifierPlugin.PluginConfig; import io.grpc.xds.Filter.FilterConfig; import io.grpc.xds.Filter.NamedFilterConfig; diff --git a/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java index 3c883414b5a..d65cf96c00d 100644 --- a/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java @@ -66,7 +66,6 @@ import io.grpc.internal.PickFirstLoadBalancerProviderAccessor; import io.grpc.internal.PickSubchannelArgsImpl; import io.grpc.testing.TestMethodDescriptors; -import io.grpc.tp.zah.XxHash64; import io.grpc.util.AbstractTestHelper; import io.grpc.util.ForwardingLoadBalancerHelper; import io.grpc.util.MultiChildLoadBalancer.ChildLbState; diff --git a/third-party/zero-allocation-hashing/LICENSE b/xds/third_party/zero-allocation-hashing/LICENSE similarity index 100% rename from third-party/zero-allocation-hashing/LICENSE rename to xds/third_party/zero-allocation-hashing/LICENSE diff --git a/third-party/zero-allocation-hashing/NOTICE b/xds/third_party/zero-allocation-hashing/NOTICE similarity index 100% rename from third-party/zero-allocation-hashing/NOTICE rename to xds/third_party/zero-allocation-hashing/NOTICE diff --git a/third-party/zero-allocation-hashing/src/main/java/io/grpc/tp/zah/XxHash64.java b/xds/third_party/zero-allocation-hashing/main/java/io/grpc/xds/XxHash64.java similarity index 93% rename from third-party/zero-allocation-hashing/src/main/java/io/grpc/tp/zah/XxHash64.java rename to xds/third_party/zero-allocation-hashing/main/java/io/grpc/xds/XxHash64.java index 69f251a1113..9ca9ebc7762 100644 --- a/third-party/zero-allocation-hashing/src/main/java/io/grpc/tp/zah/XxHash64.java +++ b/xds/third_party/zero-allocation-hashing/main/java/io/grpc/xds/XxHash64.java @@ -18,7 +18,7 @@ * Modified by the gRPC Authors */ -package io.grpc.tp.zah; +package io.grpc.xds; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; @@ -33,8 +33,8 @@ * * OpenHFT/Zero-Allocation-Hashing. */ -final public class XxHash64 { - static public final XxHash64 INSTANCE = new XxHash64(0); +final class XxHash64 { + static final XxHash64 INSTANCE = new XxHash64(0); // Primes if treated as unsigned private static final long P1 = -7046029288634856825L; @@ -47,12 +47,12 @@ final public class XxHash64 { private final long seed; private final long voidHash; - public XxHash64(long seed) { + XxHash64(long seed) { this.seed = seed; this.voidHash = finalize(seed + P5); } - public long hashLong(long input) { + long hashLong(long input) { input = byteOrder == ByteOrder.LITTLE_ENDIAN ? input : Long.reverseBytes(input); long hash = seed + P5 + 8; input *= P2; @@ -63,7 +63,7 @@ public long hashLong(long input) { return finalize(hash); } - public long hashInt(int input) { + long hashInt(int input) { input = byteOrder == ByteOrder.LITTLE_ENDIAN ? input : Integer.reverseBytes(input); long hash = seed + P5 + 4; hash ^= (input & 0xFFFFFFFFL) * P1; @@ -71,7 +71,7 @@ public long hashInt(int input) { return finalize(hash); } - public long hashShort(short input) { + long hashShort(short input) { input = byteOrder == ByteOrder.LITTLE_ENDIAN ? input : Short.reverseBytes(input); long hash = seed + P5 + 2; hash ^= (input & 0xFFL) * P5; @@ -81,22 +81,22 @@ public long hashShort(short input) { return finalize(hash); } - public long hashChar(char input) { + long hashChar(char input) { return hashShort((short) input); } - public long hashByte(byte input) { + long hashByte(byte input) { long hash = seed + P5 + 1; hash ^= (input & 0xFF) * P5; hash = Long.rotateLeft(hash, 11) * P1; return finalize(hash); } - public long hashVoid() { + long hashVoid() { return voidHash; } - public long hashAsciiString(String input) { + long hashAsciiString(String input) { ByteSupplier supplier = new AsciiStringByteSupplier(input); return hashBytes(supplier); } @@ -106,7 +106,7 @@ long hashBytes(byte[] bytes) { return hashBytes(supplier); } - public long hashBytes(byte[] bytes, int offset, int len) { + long hashBytes(byte[] bytes, int offset, int len) { ByteSupplier supplier = new PlainByteSupplier(bytes, offset, len); return hashBytes(supplier); } diff --git a/third-party/zero-allocation-hashing/src/test/java/io/grpc/tp/zah/XxHash64Test.java b/xds/third_party/zero-allocation-hashing/test/java/io/grpc/xds/XxHash64Test.java similarity index 99% rename from third-party/zero-allocation-hashing/src/test/java/io/grpc/tp/zah/XxHash64Test.java rename to xds/third_party/zero-allocation-hashing/test/java/io/grpc/xds/XxHash64Test.java index be14c6351ca..10219a2f72e 100644 --- a/third-party/zero-allocation-hashing/src/test/java/io/grpc/tp/zah/XxHash64Test.java +++ b/xds/third_party/zero-allocation-hashing/test/java/io/grpc/xds/XxHash64Test.java @@ -18,7 +18,7 @@ * Modified by the gRPC Authors */ -package io.grpc.tp.zah; +package io.grpc.xds; import static org.junit.Assert.assertEquals; From e404564a8c06d7377b78dc8af8d0c0dade689f98 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Mon, 6 Oct 2025 13:45:53 +0000 Subject: [PATCH 08/19] Changed hashing function to `murmur3_128` --- util/build.gradle | 1 - .../util/RandomSubsettingLoadBalancer.java | 27 ++++++++++++------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/util/build.gradle b/util/build.gradle index 17ed12daa59..6fbd6925c00 100644 --- a/util/build.gradle +++ b/util/build.gradle @@ -19,7 +19,6 @@ dependencies { api project(':grpc-api') implementation project(':grpc-core'), - project(':grpc-third-party:zero-allocation-hashing'), libraries.animalsniffer.annotations, libraries.guava testImplementation libraries.guava.testlib, diff --git a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java index fabf193f3af..77c6109b450 100644 --- a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java @@ -19,10 +19,14 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import com.google.common.hash.HashCode; +import com.google.common.hash.HashFunction; +import com.google.common.hash.Hashing; +import com.google.common.primitives.UnsignedBytes; import io.grpc.EquivalentAddressGroup; import io.grpc.LoadBalancer; import io.grpc.Status; -import io.grpc.tp.zah.XxHash64; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; @@ -37,13 +41,16 @@ * https://https://github.com/grpc/proposal/blob/master/A68-random-subsetting.md */ final class RandomSubsettingLoadBalancer extends LoadBalancer { + private static final Comparator BYTE_ARRAY_COMPARATOR = + UnsignedBytes.lexicographicalComparator(); + private final GracefulSwitchLoadBalancer switchLb; - private final XxHash64 hashFunc; + private final HashFunction hashFunc; public RandomSubsettingLoadBalancer(Helper helper) { switchLb = new GracefulSwitchLoadBalancer(checkNotNull(helper, "helper")); - long seed = new Random().nextLong(); - hashFunc = new XxHash64(seed); + int seed = new Random().nextInt(); + hashFunc = Hashing.murmur3_128(seed); } @Override @@ -76,7 +83,9 @@ private ResolvedAddresses filterEndpoints(ResolvedAddresses resolvedAddresses, l endpointWithHashList.add( new EndpointWithHash( addressGroup, - hashFunc.hashAsciiString(addressGroup.getAddresses().get(0).toString()))); + hashFunc.hashString( + addressGroup.getAddresses().get(0).toString(), + StandardCharsets.UTF_8))); } Collections.sort(endpointWithHashList, new HashAddressComparator()); @@ -106,18 +115,18 @@ public void shutdown() { private static final class EndpointWithHash { public final EquivalentAddressGroup addressGroup; - public final long hash; + public final HashCode hashCode; - public EndpointWithHash(EquivalentAddressGroup addressGroup, long hash) { + public EndpointWithHash(EquivalentAddressGroup addressGroup, HashCode hashCode) { this.addressGroup = addressGroup; - this.hash = hash; + this.hashCode = hashCode; } } private static final class HashAddressComparator implements Comparator { @Override public int compare(EndpointWithHash lhs, EndpointWithHash rhs) { - return Long.compare(lhs.hash, rhs.hash); + return BYTE_ARRAY_COMPARATOR.compare(lhs.hashCode.asBytes(), rhs.hashCode.asBytes()); } } From 768462953bc07ffdd21c05038fcaba47557712cf Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Mon, 6 Oct 2025 13:52:35 +0000 Subject: [PATCH 09/19] Removed leftover dependency from `BUILD.bazel` --- util/BUILD.bazel | 1 - 1 file changed, 1 deletion(-) diff --git a/util/BUILD.bazel b/util/BUILD.bazel index ef22c733511..32d5a367b95 100644 --- a/util/BUILD.bazel +++ b/util/BUILD.bazel @@ -13,7 +13,6 @@ java_library( deps = [ "//api", "//core:internal", - "//third-party/zero-allocation-hashing", artifact("com.google.code.findbugs:jsr305"), artifact("com.google.errorprone:error_prone_annotations"), artifact("com.google.guava:guava"), From 4fbf81d45b0b2a1a733d1181583d3ec46aee214a Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Tue, 7 Oct 2025 11:26:54 +0000 Subject: [PATCH 10/19] Changed hash code comparator --- .../java/io/grpc/util/RandomSubsettingLoadBalancer.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java index 77c6109b450..98201de0bd4 100644 --- a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java @@ -22,7 +22,6 @@ import com.google.common.hash.HashCode; import com.google.common.hash.HashFunction; import com.google.common.hash.Hashing; -import com.google.common.primitives.UnsignedBytes; import io.grpc.EquivalentAddressGroup; import io.grpc.LoadBalancer; import io.grpc.Status; @@ -41,9 +40,6 @@ * https://https://github.com/grpc/proposal/blob/master/A68-random-subsetting.md */ final class RandomSubsettingLoadBalancer extends LoadBalancer { - private static final Comparator BYTE_ARRAY_COMPARATOR = - UnsignedBytes.lexicographicalComparator(); - private final GracefulSwitchLoadBalancer switchLb; private final HashFunction hashFunc; @@ -126,7 +122,7 @@ public EndpointWithHash(EquivalentAddressGroup addressGroup, HashCode hashCode) private static final class HashAddressComparator implements Comparator { @Override public int compare(EndpointWithHash lhs, EndpointWithHash rhs) { - return BYTE_ARRAY_COMPARATOR.compare(lhs.hashCode.asBytes(), rhs.hashCode.asBytes()); + return Long.compare(lhs.hashCode.asLong(), rhs.hashCode.asLong()); } } From 5745d0a8c91f3adf7125f3488759971fd2c7cc47 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Tue, 7 Oct 2025 11:27:41 +0000 Subject: [PATCH 11/19] Improved test assertion to get better failure message --- .../java/io/grpc/util/RandomSubsettingLoadBalancerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java b/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java index 245f8f6bedd..50541618390 100644 --- a/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java +++ b/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java @@ -276,7 +276,7 @@ public void verifyConnectionsByServer( int maxConnections = Collections.max(connectionsByServer.values()); - assertThat(maxConnections <= expectedMaxConnections).isTrue(); + assertThat(maxConnections).isAtMost(expectedMaxConnections); } private class BackendDetails { From eb06b9e0ff5b13caf5f2ac2c2f6f258cdcbb1c8b Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Tue, 7 Oct 2025 11:29:29 +0000 Subject: [PATCH 12/19] Excluded new LB from javadoc --- util/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/util/build.gradle b/util/build.gradle index 6fbd6925c00..846b110b106 100644 --- a/util/build.gradle +++ b/util/build.gradle @@ -58,6 +58,7 @@ animalsniffer { tasks.named("javadoc").configure { exclude 'io/grpc/util/MultiChildLoadBalancer.java' exclude 'io/grpc/util/OutlierDetectionLoadBalancer*' + exclude 'io/grpc/util/RandomSubsettingLoadBalancer*' exclude 'io/grpc/util/RoundRobinLoadBalancer*' } From e1680326bc5b7123a41bd13b811c7502e61b67c5 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Tue, 7 Oct 2025 14:38:31 +0000 Subject: [PATCH 13/19] Improved `subsetSize` parsing --- .../util/RandomSubsettingLoadBalancer.java | 31 +++++++------------ .../RandomSubsettingLoadBalancerProvider.java | 2 +- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java index 98201de0bd4..748dabde80d 100644 --- a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java @@ -65,9 +65,7 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { // implements the subsetting algorithm, as described in A68: // https://github.com/grpc/proposal/pull/423 - private ResolvedAddresses filterEndpoints(ResolvedAddresses resolvedAddresses, long subsetSize) { - // configured subset sizes in the range [Integer.MAX_VALUE, Long.MAX_VALUE] will always fall - // into this if statement due to collection indexing limitations in JVM + private ResolvedAddresses filterEndpoints(ResolvedAddresses resolvedAddresses, int subsetSize) { if (subsetSize >= resolvedAddresses.getAddresses().size()) { return resolvedAddresses; } @@ -86,12 +84,8 @@ private ResolvedAddresses filterEndpoints(ResolvedAddresses resolvedAddresses, l Collections.sort(endpointWithHashList, new HashAddressComparator()); - // array is constructed for subset sizes in range [0, Integer.MAX_VALUE), therefore casting - // from long to int is not going to overflow here - ArrayList addressGroups = new ArrayList<>((int) subsetSize); + ArrayList addressGroups = new ArrayList<>(subsetSize); - // for loop is executed for subset sizes in range [0, Integer.MAX_VALUE), therefore indexing - // variable is not going to overflow here for (int idx = 0; idx < subsetSize; ++idx) { addressGroups.add(endpointWithHashList.get(idx).addressGroup); } @@ -127,25 +121,24 @@ public int compare(EndpointWithHash lhs, EndpointWithHash rhs) { } public static final class RandomSubsettingLoadBalancerConfig { - public final long subsetSize; + public final int subsetSize; public final Object childConfig; - private RandomSubsettingLoadBalancerConfig(long subsetSize, Object childConfig) { + private RandomSubsettingLoadBalancerConfig(int subsetSize, Object childConfig) { this.subsetSize = subsetSize; this.childConfig = childConfig; } public static class Builder { - Long subsetSize; + int subsetSize; Object childConfig; - public Builder setSubsetSize(Integer subsetSize) { - checkNotNull(subsetSize, "subsetSize"); - // {@code Integer.toUnsignedLong(int)} is not part of Android API level 21, therefore doing - // it manually - Long subsetSizeAsLong = ((long) subsetSize) & 0xFFFFFFFFL; - checkArgument(subsetSizeAsLong > 0L, "Subset size must be greater than 0"); - this.subsetSize = subsetSizeAsLong; + public Builder setSubsetSize(long subsetSize) { + checkArgument(subsetSize > 0L, "Subset size must be greater than 0"); + // clamping subset size to Integer.MAX_VALUE due to collection indexing limitations in JVM + long subsetSizeClamped = Math.min(subsetSize, (long) Integer.MAX_VALUE); + // safe narrowing cast due to clamping + this.subsetSize = (int) subsetSizeClamped; return this; } @@ -156,7 +149,7 @@ public Builder setChildConfig(Object childConfig) { public RandomSubsettingLoadBalancerConfig build() { return new RandomSubsettingLoadBalancerConfig( - checkNotNull(subsetSize, "subsetSize"), + subsetSize, checkNotNull(childConfig, "childConfig")); } } diff --git a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancerProvider.java b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancerProvider.java index b8c15b663cc..54680823803 100644 --- a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancerProvider.java +++ b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancerProvider.java @@ -61,7 +61,7 @@ public ConfigOrError parseLoadBalancingPolicyConfig(Map rawConfig) { } private ConfigOrError parseLoadBalancingPolicyConfigInternal(Map rawConfig) { - Integer subsetSize = JsonUtil.getNumberAsInteger(rawConfig, "subsetSize"); + Long subsetSize = JsonUtil.getNumberAsLong(rawConfig, "subsetSize"); if (subsetSize == null) { return ConfigOrError.fromError( Status.INTERNAL.withDescription( From 56d33a7ba81b58da105ee4b78a15d90cdfdd67a8 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Tue, 7 Oct 2025 14:41:56 +0000 Subject: [PATCH 14/19] Adjusted expectation for flaky test --- .../java/io/grpc/util/RandomSubsettingLoadBalancerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java b/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java index 50541618390..7e889379e0f 100644 --- a/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java +++ b/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java @@ -225,7 +225,7 @@ public void backendsCanBeDistributedEvenly_subsetting100_100_25() { // verifies: https://github.com/grpc/proposal/blob/master/A68_graphics/subsetting100-10-5.png @Test public void backendsCanBeDistributedEvenly_subsetting100_10_5() { - verifyConnectionsByServer(100, 10, 5, 65); + verifyConnectionsByServer(100, 10, 5, 70); } // verifies: https://github.com/grpc/proposal/blob/master/A68_graphics/subsetting500-10-5.png From 04c4beb5c2d4459df33b0918c32949208cc4ebf9 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Wed, 8 Oct 2025 09:31:03 +0000 Subject: [PATCH 15/19] Changed way of storing hashes --- .../grpc/util/RandomSubsettingLoadBalancer.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java index 748dabde80d..aa1d1dd1ab4 100644 --- a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java @@ -74,12 +74,10 @@ private ResolvedAddresses filterEndpoints(ResolvedAddresses resolvedAddresses, i new ArrayList<>(resolvedAddresses.getAddresses().size()); for (EquivalentAddressGroup addressGroup : resolvedAddresses.getAddresses()) { - endpointWithHashList.add( - new EndpointWithHash( - addressGroup, - hashFunc.hashString( - addressGroup.getAddresses().get(0).toString(), - StandardCharsets.UTF_8))); + HashCode hashCode = hashFunc.hashString( + addressGroup.getAddresses().get(0).toString(), + StandardCharsets.UTF_8); + endpointWithHashList.add(new EndpointWithHash(addressGroup, hashCode.asLong())); } Collections.sort(endpointWithHashList, new HashAddressComparator()); @@ -105,9 +103,9 @@ public void shutdown() { private static final class EndpointWithHash { public final EquivalentAddressGroup addressGroup; - public final HashCode hashCode; + public final long hashCode; - public EndpointWithHash(EquivalentAddressGroup addressGroup, HashCode hashCode) { + public EndpointWithHash(EquivalentAddressGroup addressGroup, long hashCode) { this.addressGroup = addressGroup; this.hashCode = hashCode; } @@ -116,7 +114,7 @@ public EndpointWithHash(EquivalentAddressGroup addressGroup, HashCode hashCode) private static final class HashAddressComparator implements Comparator { @Override public int compare(EndpointWithHash lhs, EndpointWithHash rhs) { - return Long.compare(lhs.hashCode.asLong(), rhs.hashCode.asLong()); + return Long.compare(lhs.hashCode, rhs.hashCode); } } From 91634387c3abaf4bc33a25598053a0393e325c94 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Wed, 8 Oct 2025 09:49:15 +0000 Subject: [PATCH 16/19] Improved config builder --- .../java/io/grpc/util/RandomSubsettingLoadBalancer.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java index aa1d1dd1ab4..54ad6eb92d1 100644 --- a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java @@ -18,10 +18,12 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import com.google.common.hash.HashCode; import com.google.common.hash.HashFunction; import com.google.common.hash.Hashing; +import com.google.common.primitives.Ints; import io.grpc.EquivalentAddressGroup; import io.grpc.LoadBalancer; import io.grpc.Status; @@ -134,9 +136,7 @@ public static class Builder { public Builder setSubsetSize(long subsetSize) { checkArgument(subsetSize > 0L, "Subset size must be greater than 0"); // clamping subset size to Integer.MAX_VALUE due to collection indexing limitations in JVM - long subsetSizeClamped = Math.min(subsetSize, (long) Integer.MAX_VALUE); - // safe narrowing cast due to clamping - this.subsetSize = (int) subsetSizeClamped; + this.subsetSize = Ints.saturatedCast(subsetSize); return this; } @@ -146,6 +146,7 @@ public Builder setChildConfig(Object childConfig) { } public RandomSubsettingLoadBalancerConfig build() { + checkState(subsetSize != 0L, "Subset size must be set before building the config"); return new RandomSubsettingLoadBalancerConfig( subsetSize, checkNotNull(childConfig, "childConfig")); From 9534d22de2c65963c2a351ce45f4645350784eec Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Wed, 8 Oct 2025 10:43:38 +0000 Subject: [PATCH 17/19] Updated xDS converting function --- .../io/grpc/xds/LoadBalancerConfigFactory.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/LoadBalancerConfigFactory.java b/xds/src/main/java/io/grpc/xds/LoadBalancerConfigFactory.java index c934eb5843e..40396a1b777 100644 --- a/xds/src/main/java/io/grpc/xds/LoadBalancerConfigFactory.java +++ b/xds/src/main/java/io/grpc/xds/LoadBalancerConfigFactory.java @@ -209,12 +209,12 @@ class LoadBalancerConfigFactory { * given config values. */ private static ImmutableMap buildRandomSubsettingConfig( - RandomSubsetting randomSubsetting) { + String subsetSize, ImmutableMap childConfig) { return ImmutableMap.of( RANDOM_SUBSETTING_FIELD_NAME, ImmutableMap.of( - SUBSET_SIZE, randomSubsetting.getSubsetSize(), - CHILD_POLICY_FIELD, randomSubsetting.getChildPolicy() + SUBSET_SIZE, subsetSize, + CHILD_POLICY_FIELD, ImmutableList.of(childConfig) )); } @@ -256,7 +256,7 @@ static class LoadBalancingPolicyConverter { serviceConfig = convertPickFirstConfig(typedConfig.unpack(PickFirst.class)); } else if (typedConfig.is(RandomSubsetting.class)) { serviceConfig = convertRandomSubsettingConfig( - typedConfig.unpack(RandomSubsetting.class)); + typedConfig.unpack(RandomSubsetting.class), recursionDepth); } else if (typedConfig.is(com.github.xds.type.v3.TypedStruct.class)) { serviceConfig = convertCustomConfig( typedConfig.unpack(com.github.xds.type.v3.TypedStruct.class)); @@ -349,8 +349,12 @@ static class LoadBalancingPolicyConverter { * "Converts" a random_subsetting configuration to service config format. */ private static ImmutableMap convertRandomSubsettingConfig( - RandomSubsetting randomSubsetting) { - return buildRandomSubsettingConfig(randomSubsetting); + RandomSubsetting randomSubsetting, int recursionDepth) + throws ResourceInvalidException, MaxRecursionReachedException { + return buildRandomSubsettingConfig( + randomSubsetting.getSubsetSize().toString(), + LoadBalancingPolicyConverter.convertToServiceConfig( + randomSubsetting.getChildPolicy(), recursionDepth + 1)); } /** From 94be9acb50cd3a364146c0c086cab46432ef99b0 Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Wed, 8 Oct 2025 10:52:49 +0000 Subject: [PATCH 18/19] Updated status for subset size parsing failure --- .../java/io/grpc/util/RandomSubsettingLoadBalancerProvider.java | 2 +- .../io/grpc/util/RandomSubsettingLoadBalancerProviderTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancerProvider.java b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancerProvider.java index 54680823803..fa452af3508 100644 --- a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancerProvider.java +++ b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancerProvider.java @@ -64,7 +64,7 @@ private ConfigOrError parseLoadBalancingPolicyConfigInternal(Map rawC Long subsetSize = JsonUtil.getNumberAsLong(rawConfig, "subsetSize"); if (subsetSize == null) { return ConfigOrError.fromError( - Status.INTERNAL.withDescription( + Status.UNAVAILABLE.withDescription( "Subset size missing in " + getPolicyName() + ", LB policy config=" + rawConfig)); } diff --git a/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerProviderTest.java b/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerProviderTest.java index 830ad9723d8..0809b3b4d74 100644 --- a/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerProviderTest.java +++ b/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerProviderTest.java @@ -66,7 +66,7 @@ public void parseConfigRequiresSubsetSize() throws IOException { assertThat(configOrError.getError()).isNotNull(); assertThat(configOrError.getError().toString()) .isEqualTo( - Status.INTERNAL + Status.UNAVAILABLE .withDescription("Subset size missing in random_subsetting, LB policy config={}") .toString()); } From 370570b0767c10d6070778756b776db8d494f83b Mon Sep 17 00:00:00 2001 From: Damian Zgoda Date: Wed, 8 Oct 2025 11:09:47 +0000 Subject: [PATCH 19/19] Added deterministic seed setup for LB tests --- .../io/grpc/util/RandomSubsettingLoadBalancer.java | 7 +++++++ .../grpc/util/RandomSubsettingLoadBalancerTest.java | 11 +++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java index 54ad6eb92d1..cb1d097e0b0 100644 --- a/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/RandomSubsettingLoadBalancer.java @@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; +import com.google.common.annotations.VisibleForTesting; import com.google.common.hash.HashCode; import com.google.common.hash.HashFunction; import com.google.common.hash.Hashing; @@ -51,6 +52,12 @@ public RandomSubsettingLoadBalancer(Helper helper) { hashFunc = Hashing.murmur3_128(seed); } + @VisibleForTesting + RandomSubsettingLoadBalancer(Helper helper, int seed) { + switchLb = new GracefulSwitchLoadBalancer(checkNotNull(helper, "helper")); + hashFunc = Hashing.murmur3_128(seed); + } + @Override public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { RandomSubsettingLoadBalancerConfig config = diff --git a/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java b/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java index 7e889379e0f..91dde6ba19d 100644 --- a/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java +++ b/util/src/test/java/io/grpc/util/RandomSubsettingLoadBalancerTest.java @@ -118,7 +118,8 @@ private BackendDetails setupBackends(int backendCount) { @Before public void setUp() { - loadBalancer = new RandomSubsettingLoadBalancer(mockHelper); + int seed = 0; + loadBalancer = new RandomSubsettingLoadBalancer(mockHelper, seed); int backendSize = 5; backendDetails = setupBackends(backendSize); @@ -225,7 +226,7 @@ public void backendsCanBeDistributedEvenly_subsetting100_100_25() { // verifies: https://github.com/grpc/proposal/blob/master/A68_graphics/subsetting100-10-5.png @Test public void backendsCanBeDistributedEvenly_subsetting100_10_5() { - verifyConnectionsByServer(100, 10, 5, 70); + verifyConnectionsByServer(100, 10, 5, 65); } // verifies: https://github.com/grpc/proposal/blob/master/A68_graphics/subsetting500-10-5.png @@ -252,14 +253,16 @@ public void verifyConnectionsByServer( Map connectionsByServer = Maps.newLinkedHashMap(); - for (RandomSubsettingLoadBalancerConfig config : configs) { + for (int i = 0; i < clientsCount; i++) { + RandomSubsettingLoadBalancerConfig config = configs.get(i); + ResolvedAddresses resolvedAddresses = ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.copyOf(backendDetails.servers)) .setLoadBalancingPolicyConfig(config) .build(); - loadBalancer = new RandomSubsettingLoadBalancer(mockHelper); + loadBalancer = new RandomSubsettingLoadBalancer(mockHelper, i); loadBalancer.acceptResolvedAddresses(resolvedAddresses); verify(mockChildLb, atLeastOnce()).acceptResolvedAddresses(resolvedAddrCaptor.capture());