merge #3
Merged
merge #3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It came up in #11073, and I saw it could use a little updating. Notably, I'm linking to a guide to what Git commit messages should look like. I also tried to make the language less heavy-handed and demanding.
Some addresses are equal even though their toString is different (InetSocketAddress ignores the hostname when it has an address). And some addresses are not equal even though their toString might be the same (AnonymousInProcessSocketAddress doesn't override toString()). InetSocketAddress/InetAddress do not cache the toString() result. Thus, even in the worst case that uses a HashSet, this should use less memory than the earlier approach, as no strings are formatted. It probably also significantly improves performance in the reasonably common case when an Endpoint is created just for looking up a key, because the string creation in the constructor isn't then amorized. updateChildrenWithResolvedAddresses(), for example, creates n^2 Endpoint objects for lookups.
There's no need to assume which child makes a subchannel based on the subchannel address.
* otel tracing: add binary format, grpcTraceBinContextPropagator * exception handling, use api base64 encoder omit padding remove binary format abstract class in favor of binary marshaller
A package-private class isn't visible and `@Internal` is stronger than experimental. The only way users should use WRR is via the weight_round_robin string, and that's already not suffixed with _experimental. Closes #9885
There was no point to using subchannels as keys to subchannelToReportListenerMap, as the listener is per-child. That meant the keys would be guaranteed to be known ahead-of-time and the unsynchronized getOrCreateOrcaListener() during picking was unnecessary. The picker still stores ChildLbStates to make sure that updating weights uses the correct children, but the picker itself no longer references ChildLbStates except in the constructor. That means weight calculation is moved into the LB policy, as child.getWeight() is unsynchronized, and the picker no longer needs a reference to helper.
LocalChannel is not guaranteed to be compatible with NioEventLoopGroup, and is failing with Netty 4.2.0.Alpha3-SNAPSHOT. See #11447
One LB no longer needs to extend ChildLbState and one has to start, so it is a bit of a wash. There are more LBs that need the auto-request logic, but if we have an API where subclasses override it without calling super then we can't change the implementation in the future. Adding behavior on top of a base class allows subclasses to call super, which lets the base class change over time.
That's the obvious default, and all current usages use (something equivalent to) that default.
Since 0447497 RingHashLB has not used acceptResolvedAddressesInternal(). At the time that was needed because deactivated children were part of MultiChildLB. But in 9de8e44, the logic of RingHashLB and MultiChildLB.acceptResolvedAddressesInternal() converged, so it can now swap back to using the base class for more logic.
cl/661194496
Upgrade Maven to 3.8.8, the oldest supported version, as the plugins required a newer version.
These were once needed to be overridden (e.g., by RoundRobinLB), but now nothing overrides them and MultiChildLB doesn't even call one of them.
Bazel had the dependency added because of #5046, where Guava was depending on it as compile-only and Bazel build have "unknown enum constant" warnings. Guava now has a compile dependency on j2objc, so this workaround is no longer needed. There are currently no version skew issues in Gradle, which was the only usage.
This would impact TRANSIENT_FAILURE and refreshNameResolver() logic for dual-stack endpoints.
No usages actually needed a map nor a copy.
This prevents many null checks and combines two code paths, with no additional allocations.
It will never throw, because it would only throw if helper is null, but helper is checkNotNull()ed in the constructor. It could have checked for a null return value instead; since it hasn't been, it is clear we don't need this check.
It is trivial to avoid the exception from addressIndex.getCurrentAddress(). The log message was inaccurate, as the subchannel might have been TRANSIENT_FAILURE. The only important part of the condition was whether the subchannel was the current subchannel.
requestConnection() is public API, and it is allowed to be called even if the load balancer is already READY.
It doesn't do anything. Call scheduleNextConnection() unconditionally since it is responsible for checking if `enableHappyEyeballs == true`. It's also surprising to check in the CONNECTING case but not the IDLE case.
The index was just reset by updateGroups().
…11476) GitHub began the Node16 deprecation process a year ago [1][2]. This commit updates all workflows to use the latest Node20 actions. [1]: https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/ [2]: https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/
…est client This removes the auth dependency and the implicit xds/orca from AbstractInteropTest for things that weren't used in all but one case.
This is the same as if we received a GOAWAY. We wait for the next RPC to begin connecting again. This is InternalSubchannel's behavior.
While child LB policies are unlikey to change for each cluster name (RLS returns regular cluster names, so should be unique), and the configuration for CDS policies won't change, RLS configuration can definitely change.
The child policy config should be refreshed every address update, so it shouldn't be stored in the ChildLbState. In addition, none of the current usages actually used what was stored in the ChildLbState in a meaningful way (it was always null). ResolvedAddresses was also removed from createChildLbState(), as nothing in it should be needed for creation; it varies over time and the values passed at creation are immutable.
No need to look up in the map to see if they are still a child.
This makes ClusterManagerLB more straight-forward, focusing on just the things that are relevant to it, and it avoids specialized map key handling in updateChildrenWithResolvedAddresses().
…#11495) * Determine subchannel's network locality from connected address, instead of assuming that all addresses for a subchannel are in the same locality.
Detachable lets a buffer outlive its original lifetime. The new lifetime is application-controlled. If the application fails to read/close the stream, then the leak detector wouldn't make clear what code was responsible for the buffer's lifetime. With this touch, we'll be able to see detach() was called and thus know the application needs debugging. Realized when looking at b/364531464, although I think the issue is unrelated.
unix.sh is shared by multiple OSes and environments. Clear JAVA_HOME, since we never want to use that as PATH is more reliable, better supported, and more typical.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
12 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.