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

Upgrade coroutines version to 1.6.1. #327

Merged
merged 23 commits into from
May 27, 2022
Merged

Conversation

lowasser
Copy link
Collaborator

This is a necessary start on using CopyableThreadContextElement to interact with the thread-local gRPC context.

@jamesward
Copy link
Collaborator

Looks like we are still waiting on a 1.6.1 release to hit Maven Central.

@jamesward
Copy link
Collaborator

Coroutines 1.6.1 is out and I'm trying to upgrade to it but getting two test failures in stub (working on getting CI to repro them):

io.grpc.kotlin.ServerCallsTest > clientStreamingWhenRequestsCancelledNoBackpressure FAILED
    io.grpc.StatusException: UNKNOWN
        at io.grpc.Status.asException(Status.java:551)
        at io.grpc.kotlin.ClientCalls$rpcImpl$1$1$1.onClose(ClientCalls.kt:298)
        at io.grpc.internal.ClientCallImpl.closeObserver(ClientCallImpl.java:553)
        at io.grpc.internal.ClientCallImpl.access$300(ClientCallImpl.java:68)
        at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1StreamClosed.runInternal(ClientCallImpl.java:739)
        at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1StreamClosed.runInContext(ClientCallImpl.java:718)
        at io.grpc.internal.ContextRunnable.run(ContextRunnable.java:37)
        at io.grpc.internal.SerializingExecutor.run(SerializingExecutor.java:123)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)
io.grpc.kotlin.ServerCallsTest > clientStreamingDoesntWaitForAllRequests FAILED
    io.grpc.StatusException: UNKNOWN
        at io.grpc.Status.asException(Status.java:551)
        at io.grpc.kotlin.ClientCalls$rpcImpl$1$1$1.onClose(ClientCalls.kt:298)
        at io.grpc.internal.ClientCallImpl.closeObserver(ClientCallImpl.java:553)
        at io.grpc.internal.ClientCallImpl.access$300(ClientCallImpl.java:68)
        at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1StreamClosed.runInternal(ClientCallImpl.java:739)
        at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1StreamClosed.runInContext(ClientCallImpl.java:718)
        at io.grpc.internal.ContextRunnable.run(ContextRunnable.java:37)
        at io.grpc.internal.SerializingExecutor.run(SerializingExecutor.java:123)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)

@jamesward jamesward added this to the 1.3.0 milestone Apr 5, 2022
@lowasser lowasser self-assigned this May 3, 2022
@rkklai
Copy link

rkklai commented May 9, 2022

Thanks for the work to upgrade coroutines to 1.6.1. Is there any ETA when this will be merged?

@jamesward
Copy link
Collaborator

We just need to get the Bazel build updated then will roll a new release.

build.gradle.kts Outdated Show resolved Hide resolved
@lowasser
Copy link
Collaborator Author

Looks like a flake locally. I think we can merge this.

@jamesward
Copy link
Collaborator

I can't reproduce the failure locally either and the failures kinda move around. Here is one:

io.grpc.testing.integration.Http2OkHttpTest > cancelAfterFirstResponse FAILED
    java.lang.NullPointerException
        at io.grpc.testing.integration.AbstractInteropTest.assertServerStatsTrace(AbstractInteropTest.kt:1525)
        at io.grpc.testing.integration.AbstractInteropTest.assertStatsTrace(AbstractInteropTest.kt:1486)
        at io.grpc.testing.integration.AbstractInteropTest.assertStatsTrace$default(AbstractInteropTest.kt:1479)
        at io.grpc.testing.integration.AbstractInteropTest.cancelAfterFirstResponse(AbstractInteropTest.kt:689)

And another:

io.grpc.testing.integration.Http2OkHttpTest > deadlineInPast FAILED
    java.lang.AssertionError
        at org.junit.Assert.fail(Assert.java:87)
        at org.junit.Assert.assertTrue(Assert.java:42)
        at org.junit.Assert.assertTrue(Assert.java:53)
        at io.grpc.testing.integration.AbstractInteropTest.assertClientStatsTrace(AbstractInteropTest.kt:1496)
        at io.grpc.testing.integration.AbstractInteropTest.assertStatsTrace(AbstractInteropTest.kt:1485)
        at io.grpc.testing.integration.AbstractInteropTest.assertStatsTrace$default(AbstractInteropTest.kt:1479)
        at io.grpc.testing.integration.AbstractInteropTest$deadlineInPast$1.invokeSuspend(AbstractInteropTest.kt:993)

We could put a CI retry on it but the flakiness concerns me. Why is CI different than local?

@lowasser
Copy link
Collaborator Author

I don't think I'm super surprised by flakiness in general, though since writing some of these tests I know more about how to avoid flakiness. I think we should probably go ahead and merge?

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

Successfully merging this pull request may close these issues.

5 participants