Skip to content
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

Add CI build with netty 4.2.0.Alpha snapshots / releases #11447

Open
normanmaurer opened this issue Aug 6, 2024 · 10 comments
Open

Add CI build with netty 4.2.0.Alpha snapshots / releases #11447

normanmaurer opened this issue Aug 6, 2024 · 10 comments

Comments

@normanmaurer
Copy link

As we are currently working on the next release of netty (4.2.0) we would love you to also run CI with it to ensure we not break grpc-java. In the past we unfortunately saw breaking even in 4.1.x and so would like to ensure we notice this as soon as possible.
Is this something you could do ?

As a side-note it would also be nice if you could run automatic testing with the latest of 4.1.x snapshots as well so we notice breaking early on.

@normanmaurer
Copy link
Author

/cc @ejona86

@ejona86
Copy link
Member

ejona86 commented Aug 6, 2024

For this one-off test, is 4.2.0.Alpha2 close enough, or should we really test the latest SNAPSHOT?

@normanmaurer
Copy link
Author

I mean SNAPSHOT would be best but latest Alpha release should also be fine. I will cut a new Alpha soonish as well

@ejona86
Copy link
Member

ejona86 commented Aug 7, 2024

A quick test with just Linux and Alpha2 mostly just had *EventLoopGroup warnings. We have one shading test failure, but that is unlikely to matter; it is testing that we properly rewrote META-INF/native-image files and one of them was probably renamed. (Edit: Actually, we had already fixed that on a newer version of Netty, but I was on a different base than I thought. Changing to a fresh master, everything passes, on Linux)

@normanmaurer
Copy link
Author

@ejona86 nice! Yeah the EventLoopGroup warnings are expected as constructing these directly is deprecated in favor of using MultiThreadIoEventLoopGroup(....)

@ejona86
Copy link
Member

ejona86 commented Aug 7, 2024

Yeah, I saw the warnings were expected and that we couldn't pre-migrate.

I'm trying a one-off with #11450. Early results show a channelFactoryShouldNNotSetSocketOptionKeepAlive failure on Linux which I don't yet see on my machine (Java 11). Seems like the channel creation is failing, but I don't know why that'd be flaky. I'll wait until the other CIs finish before digging too deep.

For a continuous CI (tehe) I'm not quite sure how we'd handle the warnings before the first Final release. But it also seems there's only a few places to change. Running the latest snapshot may be a bit interesting to select without manually bumping the version each month.

@normanmaurer
Copy link
Author

@ejona86 please keep me posted and let me know if there is anything you need

@normanmaurer
Copy link
Author

@ejona86 any updates on the failed tests ?

@ejona86
Copy link
Member

ejona86 commented Aug 9, 2024

The tests have passed on every platform. But the Github Actions for Java 8/17 failed. I figured out where the test results were saved. Java 17 stdout/err doesn't have anything special. But in the Java 8 failure, this looks suspicious but may be from the @After and not related to the root cause:

Aug 07, 2024 12:25:55 AM io.netty.util.concurrent.DefaultPromise safeExecute
SEVERE: Failed to submit a listener notification task. Event loop shut down?
java.util.concurrent.RejectedExecutionException: event executor terminated
	at io.netty.util.concurrent.SingleThreadEventExecutor.reject(SingleThreadEventExecutor.java:1053)
	at io.netty.util.concurrent.SingleThreadEventExecutor.offerTask(SingleThreadEventExecutor.java:399)
	at io.netty.util.concurrent.SingleThreadEventExecutor.addTask(SingleThreadEventExecutor.java:392)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:955)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute0(SingleThreadEventExecutor.java:921)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:911)
	at io.netty.util.concurrent.DefaultPromise.safeExecute(DefaultPromise.java:862)
	at io.netty.util.concurrent.DefaultPromise.notifyListeners(DefaultPromise.java:500)
	at io.netty.util.concurrent.DefaultPromise.setValue0(DefaultPromise.java:636)
	at io.netty.util.concurrent.DefaultPromise.setFailure0(DefaultPromise.java:629)
	at io.netty.util.concurrent.DefaultPromise.tryFailure(DefaultPromise.java:118)
	at io.netty.channel.nio.AbstractNioChannel.doClose(AbstractNioChannel.java:578)
	at io.netty.channel.socket.nio.NioSocketChannel.doClose(NioSocketChannel.java:342)
	at io.netty.channel.AbstractChannel$AbstractUnsafe.doClose0(AbstractChannel.java:618)
	at io.netty.channel.AbstractChannel$AbstractUnsafe.access$1000(AbstractChannel.java:288)
	at io.netty.channel.AbstractChannel$AbstractUnsafe$6.run(AbstractChannel.java:575)
	at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:148)
	at io.netty.util.concurrent.GlobalEventExecutor$TaskRunner.run(GlobalEventExecutor.java:262)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.lang.Thread.run(Thread.java:750)

Aug 07, 2024 12:25:55 AM io.netty.channel.AbstractChannel$AbstractUnsafe invokeLater
WARNING: Can't invoke task later as EventLoop rejected it
java.util.concurrent.RejectedExecutionException: event executor terminated
	at io.netty.util.concurrent.SingleThreadEventExecutor.reject(SingleThreadEventExecutor.java:1053)
	at io.netty.util.concurrent.SingleThreadEventExecutor.offerTask(SingleThreadEventExecutor.java:399)
	at io.netty.util.concurrent.SingleThreadEventExecutor.addTask(SingleThreadEventExecutor.java:392)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:955)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute0(SingleThreadEventExecutor.java:921)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:911)
	at io.netty.channel.AbstractChannel$AbstractUnsafe.invokeLater(AbstractChannel.java:888)
	at io.netty.channel.AbstractChannel$AbstractUnsafe.access$1200(AbstractChannel.java:288)
	at io.netty.channel.AbstractChannel$AbstractUnsafe$6.run(AbstractChannel.java:578)
	at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:148)
	at io.netty.util.concurrent.GlobalEventExecutor$TaskRunner.run(GlobalEventExecutor.java:262)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.lang.Thread.run(Thread.java:750)

It didn't reproduce on my machine, so I suspect a race. Looking at the test, I'm most suspicious about it using a LocalChannel with an NioEventLoopGroup (out of laziness). I can try swapping the event loop type, but it might be interesting to figure out why it is flaky.

@ejona86
Copy link
Member

ejona86 commented Aug 12, 2024

Using DefaultEventLoopGroup instead of NioEventLoopGroup with LocalChannel fixed the flake.

All the tests passed on all platforms (the errors are because an example is failing because it doesn't know how to find the snapshot; on Thursday an example was added that used grpc-netty instead of grpc-netty-shaded).

ejona86 added a commit to ejona86/grpc-java that referenced this issue Aug 12, 2024
LocalChannel is not guaranteed to be compatible with NioEventLoopGroup,
and is failing with Netty 4.2.0.Alpha3-SNAPSHOT.

See grpc#11447
ejona86 added a commit that referenced this issue Aug 12, 2024
LocalChannel is not guaranteed to be compatible with NioEventLoopGroup,
and is failing with Netty 4.2.0.Alpha3-SNAPSHOT.

See #11447
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants