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

Intermittent test failure: LazyDynamicEndpointGroupTest.emptyEndpoint() #3381

Closed
minwoox opened this issue Mar 9, 2021 · 4 comments · Fixed by #3636
Closed

Intermittent test failure: LazyDynamicEndpointGroupTest.emptyEndpoint() #3381

minwoox opened this issue Mar 9, 2021 · 4 comments · Fixed by #3636
Labels
Milestone

Comments

@minwoox
Copy link
Member

minwoox commented Mar 9, 2021

LazyDynamicEndpointGroupTest > emptyEndpoint() FAILED
    java.lang.AssertionError:
    Expecting a throwable with cause being an instance of:
     <com.linecorp.armeria.client.UnprocessedRequestException>
    but was an instance of:
     <com.linecorp.armeria.common.stream.ClosedStreamException>
        at com.linecorp.armeria.client.grpc.LazyDynamicEndpointGroupTest.emptyEndpoint(LazyDynamicEndpointGroupTest.java:91)
@minwoox
Copy link
Member Author

minwoox commented Mar 9, 2021

Let me close this issue because the exception is a ClosedStreamException. 😅

@minwoox minwoox closed this as completed Mar 9, 2021
@trustin
Copy link
Collaborator

trustin commented Mar 26, 2021

Saw this again at: https://ci.appveyor.com/project/line/armeria/builds/38375545/job/rvkjlg7l0dfp9b0t#L488

@trustin trustin reopened this Mar 26, 2021
@trustin trustin added this to the 1.6.0 milestone Mar 26, 2021
@trustin trustin added the defect label Mar 26, 2021
@minwoox
Copy link
Member Author

minwoox commented Mar 26, 2021

Yeah, it shouldn't be a ClosedStreamException.

@minwoox minwoox modified the milestones: 1.6.0, 1.7.0 Apr 1, 2021
@trustin trustin modified the milestones: 1.7.0, 1.8.0 Apr 27, 2021
@trustin
Copy link
Collaborator

trustin commented May 11, 2021

@ikhoon ikhoon modified the milestones: 1.8.0, 1.9.0 May 14, 2021
ikhoon added a commit to ikhoon/armeria that referenced this issue Jun 15, 2021
…izing a context

Motivation:

`LazyDynamicEndpointGroupTest.emptyEndpoint()` sometimes failed in CI
builds. line#3381 It expects to fail the test with `EmptyEndpointException`,
however it fails with `ClosedStreamException`.

After digging the issue, I find out that there is a race in
`whenInitialized`.  If the `acquiredEventLoop.execute()` is
executed immediately and eailer than returning the method,
https://github.com/line/armeria/blob/d4880fe12690d2dafd2c5e7fa9f24c3b24837a00/core/src/main/java/com/linecorp/armeria/client/DefaultClientRequestContext.java#L307-L309
the callbacks of `ctx.whenInitialized()` will be invoked
before a `RequestLog` is completed.
https://github.com/line/armeria/blob/207c5e038f59802dca769936a50e219a5fe308ea/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaClientCall.java#L337-L348
As the `req` is closed already, the `req.write()` would
be failed with `ClosedStreamException`.

Modifications:

- Complete `DefaultClientRequestContext.whenIntialized()` after
  `initContextAndExecuteWithFallback` of `ClientUtil`

Result:

- You no longer see `ClosedStreamException` when an `EndpointGroup` is empty.
- Fixes line#3381
trustin pushed a commit that referenced this issue Jun 18, 2021
…nitializing a context (#3636)

Motivation:

`LazyDynamicEndpointGroupTest.emptyEndpoint()` sometimes failed in CI
builds. #3381 It expects to fail the test with `EmptyEndpointException`,
however, it fails with `ClosedStreamException`.

After digging the issue, I find out that there is a race in `whenInitialized`.
If the `acquiredEventLoop.execute()` is executed immediately and completed earlier than
returning the method,
https://github.com/line/armeria/blob/d4880fe12690d2dafd2c5e7fa9f24c3b24837a00/core/src/main/java/com/linecorp/armeria/client/DefaultClientRequestContext.java#L307-L309
the callbacks of `ctx.whenInitialized()` will be invoked before a `RequestLog`
is completed.
https://github.com/line/armeria/blob/207c5e038f59802dca769936a50e219a5fe308ea/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaClientCall.java#L337-L348
As the `req` is closed already, the `req.write()` would be failed with
`ClosedStreamException`.

Modifications:

- Complete `DefaultClientRequestContext.whenIntialized()` after
  `initContextAndExecuteWithFallback` of `ClientUtil`

Result:

- You no longer see `ClosedStreamException` when an `EndpointGroup` is empty.
- Fixes #3381
heowc pushed a commit to heowc/armeria that referenced this issue Jun 24, 2021
…nitializing a context (line#3636)

Motivation:

`LazyDynamicEndpointGroupTest.emptyEndpoint()` sometimes failed in CI
builds. line#3381 It expects to fail the test with `EmptyEndpointException`,
however, it fails with `ClosedStreamException`.

After digging the issue, I find out that there is a race in `whenInitialized`.
If the `acquiredEventLoop.execute()` is executed immediately and completed earlier than
returning the method,
https://github.com/line/armeria/blob/d4880fe12690d2dafd2c5e7fa9f24c3b24837a00/core/src/main/java/com/linecorp/armeria/client/DefaultClientRequestContext.java#L307-L309
the callbacks of `ctx.whenInitialized()` will be invoked before a `RequestLog`
is completed.
https://github.com/line/armeria/blob/207c5e038f59802dca769936a50e219a5fe308ea/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaClientCall.java#L337-L348
As the `req` is closed already, the `req.write()` would be failed with
`ClosedStreamException`.

Modifications:

- Complete `DefaultClientRequestContext.whenIntialized()` after
  `initContextAndExecuteWithFallback` of `ClientUtil`

Result:

- You no longer see `ClosedStreamException` when an `EndpointGroup` is empty.
- Fixes line#3381
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants