Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
13e6d52
autosharding: Add module build configuration and protobuf definitions
shivaspeaks Aug 26, 2026
04c6963
add import.sh
shivaspeaks Aug 26, 2026
d161705
autosharding: Move proto and import.sh to third_party/autosharding di…
shivaspeaks Sep 1, 2026
74bdfd1
autosharding: Add SliceMap and AutoShardingPicker
shivaspeaks Sep 1, 2026
2488b37
Merge branch 'master' of https://github.com/grpc/grpc-java into autos…
shivaspeaks Sep 2, 2026
e5b4b22
grfc updated
shivaspeaks Sep 2, 2026
7dfdf6b
autosharding: Return primitive int from SliceMap.lookup to eliminate …
shivaspeaks Sep 4, 2026
67abc20
use UnsignedBytes.lexicographicalComparator()
shivaspeaks Sep 4, 2026
789de22
use ImmutableList for endpoints
shivaspeaks Sep 4, 2026
426621f
add javadoc
shivaspeaks Sep 4, 2026
1b63d20
improvements
shivaspeaks Sep 4, 2026
0771433
have exitIdler functional interface
shivaspeaks Sep 4, 2026
1db8017
create Metadata.Key statically
shivaspeaks Sep 4, 2026
29957b6
add FunctionalInterface ThreadSafeRandom
shivaspeaks Sep 4, 2026
4105fd8
fast path
shivaspeaks Sep 4, 2026
be31260
javadoc
shivaspeaks Sep 7, 2026
b99fa0c
javadoc
shivaspeaks Sep 7, 2026
12765cf
javadoc and unit test
shivaspeaks Sep 7, 2026
903ec88
context specific error
shivaspeaks Sep 7, 2026
bb9a900
clone start key
shivaspeaks Sep 7, 2026
c11754e
add some behavioural unit tests
shivaspeaks Sep 7, 2026
c79cc59
autosharding: implementation of EndpointMap and LazyChildLB
shivaspeaks Sep 8, 2026
c961edf
Merge branch 'master' of https://github.com/grpc/grpc-java into autos…
shivaspeaks Sep 8, 2026
e350685
call request conn in parent policy
shivaspeaks Sep 8, 2026
ca8fd50
use exact same indices in toPickerEndpoints
shivaspeaks Sep 8, 2026
3a3f667
clear resources in shutdown
shivaspeaks Sep 8, 2026
bbf9a6f
reset connectingScheduled flag in exitIdle
shivaspeaks Sep 8, 2026
f47816f
update unit test
shivaspeaks Sep 8, 2026
fa76449
util: refactor LazyLoadBalancer into util to use it in autosharding
shivaspeaks Sep 9, 2026
bf6eda3
Merge branch 'move-lazy-load-balancer-to-util' into autosharding-part…
shivaspeaks Sep 9, 2026
07b604e
Merge branch 'master' of https://github.com/grpc/grpc-java into autos…
shivaspeaks Sep 10, 2026
d32c495
refactor to use LazyLB
shivaspeaks Sep 10, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
257 changes: 257 additions & 0 deletions autosharding/src/main/java/io/grpc/autosharding/EndpointMap.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,257 @@
/*
* Copyright 2026 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.autosharding;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static io.grpc.ConnectivityState.IDLE;

import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableList;
import io.grpc.Attributes;
import io.grpc.ConnectivityState;
import io.grpc.EquivalentAddressGroup;
import io.grpc.LoadBalancer;
import io.grpc.LoadBalancer.FixedResultPicker;
import io.grpc.LoadBalancer.Helper;
import io.grpc.LoadBalancer.PickResult;
import io.grpc.LoadBalancer.ResolvedAddresses;
import io.grpc.LoadBalancer.SubchannelPicker;
import io.grpc.util.ForwardingLoadBalancerHelper;
import io.grpc.util.LazyLoadBalancer;
import java.util.Collection;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import javax.annotation.Nullable;
import javax.annotation.concurrent.NotThreadSafe;

/**
* Manages the mapping from endpoint hostname to {@link EndpointHolder} and coordinates
* child load balancer lifecycle and connectivity state updates.
*
* <p>Threading model: This class is not thread-safe. All methods must be invoked from the
* {@link io.grpc.SynchronizationContext} by the parent load balancer.
*/
@NotThreadSafe
final class EndpointMap {
private final Map<String, EndpointHolder> map = new LinkedHashMap<>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

variable name seems too generic.


@Nullable
EndpointHolder get(String hostname) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should to add javadocs for all package private methods, assuming they are package private because they need to be used by other classes in this package i.e. they represent an API surface. If not, let's make them private.

return map.get(checkNotNull(hostname, "hostname"));
}

void put(String hostname, EndpointHolder holder) {
map.put(checkNotNull(hostname, "hostname"), checkNotNull(holder, "holder"));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overuse of checknotNull throughout the PR makes it very difficult to read.
go/java-practices/null#tolerant

}

@Nullable
EndpointHolder remove(String hostname) {
return map.remove(checkNotNull(hostname, "hostname"));
}

Collection<EndpointHolder> values() {
return map.values();
}

Set<String> keySet() {
return map.keySet();
}

int size() {
return map.size();
}

boolean isEmpty() {
return map.isEmpty();
}

void clear() {
map.clear();
}

/**
* Re-assigns contiguous 0-based index values across all current endpoint holders.
*/
void reindex() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this method? Is the user expected to call this before converting to PickerEndpoints? Even if that's the case, if caller removes things from the map and then calls get , won't the caller end up wih endpoint state with invalid indices?

We need to really think more about defining the overall abstraction for EndpointMap.
If we need invariants, it cannot trivially provide all map functionalities unless we plan to call reindex everytim at the cost of performance.

int nextIdx = 0;
for (EndpointHolder holder : map.values()) {
holder.setIndex(nextIdx++);
}
}

/**
* Shuts down all child load balancers and clears the map.
*/
void shutdownAll() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need both shutdownAll and clear ?

for (EndpointHolder holder : map.values()) {
holder.shutdown();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the control plane operation we shutdown map , which shuts holder , which shuts childLb.

But threre could be a race between control plane shutdown and the data plane exitIdle in the RPC path? An I correct? Is our PickerEndpoint truly immutable?

}
map.clear();
}

/**
* Builds an immutable snapshot list of {@link PickerEndpoint}s placed strictly at their
* corresponding {@link EndpointHolder#getIndex()} positions.
*
* @throws IllegalStateException if endpoint indices are not contiguous from 0 to N-1
*/
ImmutableList<PickerEndpoint> toPickerEndpoints() {
int size = map.size();
if (size == 0) {
return ImmutableList.of();
}
PickerEndpoint[] array = new PickerEndpoint[size];
for (EndpointHolder holder : map.values()) {
int idx = holder.getIndex();
checkState(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this is what my previous comment was talking about. The class is currently very easy to use incorrectly and trigger these exceptions.

GIven that this runs in the synchronization context, this could break quite a lot of things .

idx >= 0 && idx < size,
"Endpoint holder index %s is out of bounds for size %s",
idx,
size);
checkState(
array[idx] == null,
"Duplicate endpoint holder index %s detected",
idx);
array[idx] = holder.toPickerEndpoint();
}
return ImmutableList.copyOf(array);
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("map", map)
.toString();
}

/**
* Holds the connectivity state, picker, and lazy child load balancer for a single endpoint.
*/
static final class EndpointHolder {
private int index;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to discuss more about the need for index.
The current implementation makes it feel like a derivable property. reindex simply iterates and assigns new index.
So, technically speaking index can be eliminated from endpoint holder and we can simply iterate over the map and push to arraylist?

private final LazyLoadBalancer childLb;
private final AtomicBoolean connectingScheduled = new AtomicBoolean(false);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why AtomicBoolean? Is this class supposed to be threadsafe? Seems counterintuitive if it's supposed to be held in a class that's not threadsafe, unless we expect people to get it from the map which may not be the correct abstraction for EndpointHolder.

private final Helper helper;
private ConnectivityState state = IDLE;
private SubchannelPicker picker = new FixedResultPicker(PickResult.withNoResult());

EndpointHolder(
int index,
Helper helper,
LoadBalancer.Factory pickFirstFactory,
@Nullable Runnable stateUpdateCallback) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anything that we want to document here about what we expect from the callback?

this.index = index;
this.helper = checkNotNull(helper, "helper");
this.childLb = new LazyLoadBalancer(
new ChildHelper(helper, stateUpdateCallback),
checkNotNull(pickFirstFactory, "pickFirstFactory"));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: We may want to move LazyLoadBalancer to lbProvider instead of lbFactory I guess? Not necessarily as a part of this PR, but something to keep in mind.

}

int getIndex() {
return index;
}

void setIndex(int index) {
this.index = index;
}

ConnectivityState getState() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I don't know how it's supposed to be used, I can't say much, but it seems like we've created a getter for each and every fields, making this field like a struct rather than class that needs to enforce invariants.

Do we need everything here?

return state;
}

SubchannelPicker getPicker() {
return picker;
}

LazyLoadBalancer getChildLb() {
return childLb;
}

PickerEndpoint toPickerEndpoint() {
return new PickerEndpoint(state, picker, this::exitIdle);
}

private void exitIdle() {
if (connectingScheduled.compareAndSet(false, true)) {
helper.getSynchronizationContext().execute(() -> {
connectingScheduled.set(false);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, we move atomic from false to true, then we execute stuff on synccontext , but then we set it to false again before requesting connection. So, this means while we are requesting connection, other rpcs can again trigger requesting connection?

What are we tryting to achieve here? This seems to be doing nothing. Shouldn't this be set to false only when the connection becomes idle again instead of before we start requesting connection?

childLb.requestConnection();
});
}
}

void updateAddresses(List<EquivalentAddressGroup> eags, Attributes attributes) {
ResolvedAddresses childAddresses = ResolvedAddresses.newBuilder()
.setAddresses(ImmutableList.copyOf(checkNotNull(eags, "eags")))
.setAttributes(checkNotNull(attributes, "attributes"))
.build();
childLb.acceptResolvedAddresses(childAddresses);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we are swallowing errors from childLb here, intentional?

}

void requestConnection() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this if we already have exitIdle? This seems like a very unsafe verision which doesn't bother executing on sync context and doesn't check the atomic boolean.

childLb.requestConnection();
}

void shutdown() {
childLb.shutdown();
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("index", index)
.add("state", state)
.add("childLb", childLb)
.toString();
}

private final class ChildHelper extends ForwardingLoadBalancerHelper {
private final Helper delegateHelper;
@Nullable private final Runnable stateUpdateCallback;

ChildHelper(Helper delegateHelper, @Nullable Runnable stateUpdateCallback) {
this.delegateHelper = checkNotNull(delegateHelper, "delegateHelper");
this.stateUpdateCallback = stateUpdateCallback;
}

@Override
protected Helper delegate() {
return delegateHelper;
}

@Override
public void updateBalancingState(ConnectivityState newState, SubchannelPicker newPicker) {
state = checkNotNull(newState, "newState");
picker = checkNotNull(newPicker, "newPicker");
if (stateUpdateCallback != null) {
stateUpdateCallback.run();
}
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("delegateHelper", delegateHelper)
.toString();
}
}
}
}
Loading
Loading