core: Remove builder class signature rewriter by sergiitk · Pull Request #10406 · grpc/grpc-java

sergiitk · 2023-07-22T00:27:50Z

It's been long enough. Removes temporary ABI hack core: Rewrite builder class signatures to avoid internal class #7834.
Removes AbstractManagedChannelImplBuilder and AbstractServerImplBuilder
Introduces ForwardingChannelBuilder2, see details in Tracking Issue for ForwardingChannelBuilder2 being Experimental #10585
Closes Hide AbstractManagedChannelImplBuilder from public API #7211
Fixes javadoc — makes the docs for the inherited methods in the channel and server builders visible:
NettyChannelBuilder before and after:

NettyServerBuilder before and after:

ABI BREAKING!

This breaks the ABI of the classes listed below.

Users that recompiled their code using grpc-java v1.36.0 (released on Feb 23, 2021) and later, ARE NOT AFFECTED.

Users that compiled their source using grpc-java earlier than v1.36.0 need to recompile when upgrading to grpc-java v1.59.0. Otherwise the code will fail on runtime with NoSuchMethodError. For example, code:

NettyChannelBuilder.forTarget("localhost:100").maxRetryAttempts(2);

Will fail with

java.lang.NoSuchMethodError: 'io.grpc.internal.AbstractManagedChannelImplBuilder io.grpc.netty.NettyChannelBuilder.maxRetryAttempts(int)'

Server builder example: code

NettyServerBuilder.forPort(80).directExecutor();

Will fail with

java.lang.NoSuchMethodError: 'io.grpc.internal.AbstractServerImplBuilder io.grpc.netty.NettyServerBuilder.directExecutor()'

Affected classes

Class AbstractManagedChannelImplBuilder is deleted, and no longer in the class hierarchy of the channel builders:

io.grpc.netty.NettyChannelBuilder
io.grpc.okhttp.OkhttpChannelBuilder
grpc.cronet.CronetChannelBuilder

Class AbstractServerImplBuilder is deleted, and no longer in the class hierarchy of the server builders:

io.grpc.netty.NettyServerBuilder
io.grpc.inprocess.InProcessServerBuilder

Related work

Hide `AbstractManagedChannelImplBuilder` — initial attempt

ABI Backward Compatibility Support

Issue Binary backwards-incompatibily in 1.33.0 #7552
- Fixed in PR Fix builders ABI backward compatibility broken in v1.33.0 #7564
- Gist: gRPC-NettyChannelBuilder-ABI-demo.md - gRPC NettyChannelBuilder binary backwards-incompatibily demonstration
- Released in v1.33.1
PR core: Rewrite builder class signatures to avoid internal class #7834
- Released in v1.36.0
Issue maxInboundMessageSize is not applied when app code has newer gRPC version than used in library #8313
- Fixed in PR Fix AbstractManagedChannelImplBuilder#maxInboundMessageSize(int) ABI #8607
- Released in v1.42.0
Fixes sometimes build fails with "Execution failed for task ':grpc-core:compileJava'. > assert i != -1" #8611

api/src/main/java/io/grpc/ForwardingChannelBuilder.java

This breaks the ABI of the classes listed below. Users that recompiled their code using grpc-java [`v1.36.0`] (https://github.com/grpc/grpc-java/releases/tag/v1.36.0) (released on Feb 23, 2021) and later, ARE NOT AFFECTED. Users that compiled their source using grpc-java earlier than [`v1.36.0`] (https://github.com/grpc/grpc-java/releases/tag/v1.36.0) need to recompile when upgrading to grpc-java `v1.59.0`. Otherwise the code will fail on runtime with `NoSuchMethodError`. For example, code: ```java NettyServerBuilder.forPort(80).directExecutor(); ``` Will fail with > `java.lang.NoSuchMethodError: 'io.grpc.internal.AbstractServerImplBuilder io.grpc.netty.NettyServerBuilder.directExecutor()'` **Affected classes** Class `AbstractServerImplBuilder` is deleted, and no longer in the class hierarchy of the server builders: - `io.grpc.netty.NettyServerBuilder` - `io.grpc.inprocess.InProcessServerBuilder`

This breaks the ABI of the classes listed below. Users that recompiled their code using grpc-java [`v1.36.0`] (https://github.com/grpc/grpc-java/releases/tag/v1.36.0) (released on Feb 23, 2021) and later, ARE NOT AFFECTED. Users that compiled their source using grpc-java earlier than [`v1.36.0`] (https://github.com/grpc/grpc-java/releases/tag/v1.36.0) need to recompile when upgrading to grpc-java `v1.59.0`. Otherwise the code will fail on runtime with `NoSuchMethodError`. For example, code: ```java NettyChannelBuilder.forTarget("localhost:100").maxRetryAttempts(2); ``` Will fail with > `java.lang.NoSuchMethodError: 'io.grpc.internal.AbstractManagedChannelImplBuilder io.grpc.netty.NettyChannelBuilder.maxRetryAttempts(int)'` **Affected classes** Class `AbstractManagedChannelImplBuilder` is deleted, and no longer in the class hierarchy of the channel builders: - `io.grpc.netty.NettyChannelBuilder` - `io.grpc.okhttp.OkhttpChannelBuilder` - `grpc.cronet.CronetChannelBuilder`

sergiitk · 2023-10-03T17:04:12Z

For posterity, here's some tests assets you can use to verify: https://drive.google.com/file/d/1mCHbBbEfz5ku8jEDyUiv6ULfqJ3flhQj/view?usp=share_link

package org.sergii.playground.netty.abi;

import io.grpc.netty.NettyChannelBuilder;

public class Main {

  public static void main(String[] args) {
    NettyChannelBuilder ncb = NettyChannelBuilder.forTarget("localhost:100");

    try {
      // Moved from AbstractManagedChannelImplBuilder to NettyChannelBuilder in 1.33.0
      ncb.maxInboundMessageSize(100);
    } catch (Throwable e) {
      System.out.println(e);
    }

    try {
      // maxRetryAttempts:
      //  <1.33.0: Inherited from AbstractManagedChannelImplBuilder
      //  =1.33.0: Inherited from ForwardingChannelBuilder
      // >=1.33.1: Inherited from tmp AbstractManagedChannelImplBuilder
      // >=1.59.0: Inherited ForwardingChannelBuilder2
      ncb.maxRetryAttempts(200);
    } catch (Throwable e) {
      System.out.println(e);
    }

    // note: maxInboundMetadataSize is the only method AbstractManagedChannelImplBuilder inherits
    //       from ManagedChannelBuilder.

    System.out.println(ncb);
  }
}

<`v1.59.x`

# v1.32.3 + v1.58.0
❯ java -cp "./builders-abi-1.32.3.jar:./grpc-1.58.0/*" org.sergii.playground.netty.abi.Main
NettyChannelBuilder{delegate=io.grpc.internal.ManagedChannelImplBuilder@1460a8c0}

# v1.35.0 + v1.58.0
❯ java -cp "./builders-abi-1.35.0.jar:./grpc-1.58.0/*" org.sergii.playground.netty.abi.Main
NettyChannelBuilder{delegate=io.grpc.internal.ManagedChannelImplBuilder@1460a8c0}

# v1.36.0 + v1.58.0
❯ java -cp "./builders-abi-1.36.0.jar:./grpc-1.58.0/*" org.sergii.playground.netty.abi.Main
NettyChannelBuilder{delegate=io.grpc.internal.ManagedChannelImplBuilder@1460a8c0}

>=`v1.59.x` BREAKING

# v1.32.3 + v1.59.x
❯ java -cp "./builders-abi-1.32.3.jar:./grpc-1.59.0-SNAPSHOT/*" org.sergii.playground.netty.abi.Main
java.lang.NoSuchMethodError: 'io.grpc.internal.AbstractManagedChannelImplBuilder io.grpc.netty.NettyChannelBuilder.maxInboundMessageSize(int)'
java.lang.NoSuchMethodError: 'io.grpc.internal.AbstractManagedChannelImplBuilder io.grpc.netty.NettyChannelBuilder.maxRetryAttempts(int)'
NettyChannelBuilder{delegate=io.grpc.internal.ManagedChannelImplBuilder@1460a8c0}

# v1.35.0 + v1.59.x
❯ java -cp "./builders-abi-1.35.0.jar:./grpc-1.59.0-SNAPSHOT/*" org.sergii.playground.netty.abi.Main
java.lang.NoSuchMethodError: 'io.grpc.internal.AbstractManagedChannelImplBuilder io.grpc.netty.NettyChannelBuilder.maxRetryAttempts(int)'
NettyChannelBuilder{delegate=io.grpc.internal.ManagedChannelImplBuilder@1460a8c0}

# v1.36.0 + v1.59.x
❯ java -cp "./builders-abi-1.36.0.jar:./grpc-1.59.0-SNAPSHOT/*" org.sergii.playground.netty.abi.Main
NettyChannelBuilder{delegate=io.grpc.internal.ManagedChannelImplBuilder@1460a8c0}

sergiitk · 2023-10-03T17:18:15Z

cc @njhill

joroKr21 · 2023-11-21T21:15:14Z

If it's breaking binary backwards compatibility it should be a new major version.
Otherwise we get java.lang.ClassNotFoundException: io.grpc.internal.AbstractManagedChannelImplBuilder

sergiitk · 2023-11-21T22:52:59Z

@joroKr21 Only classes marked with @ExperimentalApi were broken. We could've kept them broken (#7552) three years ago, when the original change breaking ABI was made. But we went out of our way to provide a stop-gap solution (#7834), allowing our users nearly two years to rebuild their code.

joroKr21 · 2023-11-21T23:24:14Z

Hmm ok, but I don't think I'm using any experimental APIs.
Looking at a stack trace, it seems to be coming from ManagedChannelBuilder.forAddress which is not experimental.

java.lang.NoClassDefFoundError: io/grpc/internal/AbstractManagedChannelImplBuilder
	at java.base/java.lang.ClassLoader.defineClass1(Native Method)
	at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1017)
	at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:150)
	at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:862)
	at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:760)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:681)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:639)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:525)
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:375)
	at io.grpc.ManagedChannelRegistry.getHardCodedClasses(ManagedChannelRegistry.java:142)
	at io.grpc.ManagedChannelRegistry.getDefaultRegistry(ManagedChannelRegistry.java:103)
	at io.grpc.ManagedChannelProvider.provider(ManagedChannelProvider.java:43)
	at io.grpc.ManagedChannelBuilder.forAddress(ManagedChannelBuilder.java:44)

ejona86 · 2023-11-21T23:46:12Z

@joroKr21, that's probably resolved if you upgrade grpc-netty/grpc-netty-shaded/grpc-okhttp (whichever you are using) to v1.59.0+.

joroKr21 · 2023-11-21T23:58:43Z

Ok - this is all coming from transitive dependencies, but can I assume that:

grpc-netty/grpc-netty-shaded/grpc-okhttp >= v1.59.0 is compatible with grpc-core < v1.59.0
grpc-netty/grpc-netty-shaded/grpc-okhttp < v1.59.0 is not compatible with grpc-core >= v1.59.0
grpc-netty/grpc-netty-shaded/grpc-okhttp < v1.59.0 is compatible with grpc-core < v1.59.0
grpc-netty/grpc-netty-shaded/grpc-okhttp >= v1.59.0 is compatible with grpc-core >= v1.59.0

So the only problem is when core is higher version than netty etc?

ejona86 · 2023-11-22T00:15:09Z

grpc-okhttp uses internal APIs of grpc-core (which is only internal APIs these days). So technically the only thing guaranteed to work is using the same version. grpc-bom can make that easier. Previously we used version pinning (e.g., [1.59.0]) to have Maven complain if there was a skew, but Maven wasn't reliable and Gradle didn't consider that an error, so it was removed in v1.56.0.

Things have happened to be compatible for a while if you do not downgrade dependencies. So grpc-core could be newer than grpc-okhttp and things have worked fine for a while it seems. That's because we are generally adding new things, and not removing stuff.

This specific instance we removed something from grpc-core, so yeah, upgrading it is what caused the problem.

sergiitk force-pushed the undo-magic-magic-methods branch 6 times, most recently from b30c7ca to e6a1b0c Compare September 11, 2023 20:03

sergiitk force-pushed the undo-magic-magic-methods branch 2 times, most recently from e1ab6d6 to 07688f5 Compare October 2, 2023 21:47

sergiitk mentioned this pull request Oct 2, 2023

Tracking Issue for ForwardingChannelBuilder2 being Experimental #10585

Closed

sergiitk marked this pull request as ready for review October 2, 2023 21:51

sergiitk requested a review from ejona86 October 2, 2023 21:51

sergiitk mentioned this pull request Oct 3, 2023

sometimes build fails with "Execution failed for task ':grpc-core:compileJava'. > assert i != -1" #8611

Closed

ejona86 approved these changes Oct 3, 2023

View reviewed changes

api/src/main/java/io/grpc/ForwardingChannelBuilder.java Outdated Show resolved Hide resolved

sergiitk added 2 commits October 3, 2023 09:45

sergiitk force-pushed the undo-magic-magic-methods branch from 07688f5 to 3eaa4c0 Compare October 3, 2023 16:47

sergiitk merged commit 0e03654 into grpc:master Oct 3, 2023

sergiitk deleted the undo-magic-magic-methods branch October 3, 2023 17:35

joroKr21 mentioned this pull request Nov 22, 2023

Set versionScheme to PVP in pom.xml for Scala users #10694

Closed

github-actions bot locked as resolved and limited conversation to collaborators Feb 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core: Remove builder class signature rewriter#10406

core: Remove builder class signature rewriter#10406
sergiitk merged 2 commits intogrpc:masterfrom
sergiitk:undo-magic-magic-methods

sergiitk commented Jul 22, 2023 •

edited

Loading

Uh oh!

Uh oh!

sergiitk commented Oct 3, 2023 •

edited

Loading

Uh oh!

sergiitk commented Oct 3, 2023

Uh oh!

joroKr21 commented Nov 21, 2023

Uh oh!

sergiitk commented Nov 21, 2023 •

edited

Loading

Uh oh!

joroKr21 commented Nov 21, 2023 •

edited

Loading

Uh oh!

ejona86 commented Nov 21, 2023

Uh oh!

joroKr21 commented Nov 21, 2023

Uh oh!

ejona86 commented Nov 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

sergiitk commented Jul 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ABI BREAKING!

Affected classes

Related work

Hide AbstractManagedChannelImplBuilder — initial attempt

ABI Backward Compatibility Support

Uh oh!

Uh oh!

sergiitk commented Oct 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

<v1.59.x

>=v1.59.x BREAKING

Uh oh!

sergiitk commented Oct 3, 2023

Uh oh!

joroKr21 commented Nov 21, 2023

Uh oh!

sergiitk commented Nov 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joroKr21 commented Nov 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ejona86 commented Nov 21, 2023

Uh oh!

joroKr21 commented Nov 21, 2023

Uh oh!

ejona86 commented Nov 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sergiitk commented Jul 22, 2023 •

edited

Loading

Hide `AbstractManagedChannelImplBuilder` — initial attempt

sergiitk commented Oct 3, 2023 •

edited

Loading

<`v1.59.x`

>=`v1.59.x` BREAKING

sergiitk commented Nov 21, 2023 •

edited

Loading

joroKr21 commented Nov 21, 2023 •

edited

Loading