-
Notifications
You must be signed in to change notification settings - Fork 3.9k
A68 random subsetting #12377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
A68 random subsetting #12377
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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", | ||
], | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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. | ||
* | ||
* <p>This implements random subsetting gRFC: | ||
* https://https://github.com/grpc/proposal/blob/master/A68-random-subsetting.md | ||
*/ | ||
@Internal | ||
public final class RandomSubsettingLoadBalancer extends LoadBalancer { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. package-private? I don't see a need for it to be public. |
||
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()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The same seed should be used for the life of the policy. When the addresses don't change much, we don't want the chosen subset to change much. If we change the seed every time, we'll always get vastly different results, even with the same input. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're not cryptographically hashing, so |
||
|
||
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<EndpointWithHash> 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<EquivalentAddressGroup> addressGroups = new ArrayList<>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pass |
||
|
||
// 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<EndpointWithHash> { | ||
@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")); | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to create a new artifact for this; that causes a lot of head-ache and lasts forever. xxhash is small enough that I'd sooner move it to grpc-core. The other option would be to use
com.google.common.hash.Hashing
, probablymurmur3_128
. xxhash has the zero-allocation implementation quite fitting for our use-case, but Guava's would probably be fine. We can always change it in the future.