Skip to content

Conversation

@glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Jul 25, 2019

…rame

Motivation:

It is possible for a connection to be immediately closed by the remote
after a successful TLS handshake. In this case, the connection would be
marked as ready and any backoff would be reset. That is, a reconnection
attempt would be made immediately and the cycle would repeat. The gRPC
core lib suggests resetting backoff after the initial settings frame has
been made.

Modifications:

Add a handler which on observing the initial settings frame:

  1. marks the connectivity as ready and,
  2. removes itself from the pipeline.

The client connection diagram was also updated to reflect this.

Result:

The connectivity state will only be marked as ready once the settings
frame has been received from the peer.

@glbrntt
Copy link
Collaborator Author

glbrntt commented Jul 25, 2019

Fixes #519

Copy link
Collaborator

@MrMage MrMage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this something we should have tests for?

/// ▲ |
/// HTTP2Frame│ │HTTP2Frame
/// ┌─┴───────────────────────▼─┐
/// ┌──────────────────────────┐
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also add the source files for these diagrams to version control at some point?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is it I'm afraid! I created them with Monodraw originally and have just updated them manually since 😬

Pretty sure you can paste ASCII-art back into Monodraw though so it shouldn't be a problem.

…rame

Motivation:

It is possible for a connection to be immediately closed by the remote
after a successful TLS handshake. In this case, the connection would be
marked as ready and any backoff would be reset. That is, a reconnection
attempt would be made immediately and the cycle would repeat. The gRPC
core lib suggests resetting backoff after the initial settings frame has
been made.

Modifications:

Add a handler which on observing the initial settings frame:
1. marks the connectivity as ready and,
2. removes itself from the pipeline.

The client connection diagram was also updated to reflect this.

Result:

The connectivity state will only be marked as ready once the settings
frame has been received from the peer.
@glbrntt
Copy link
Collaborator Author

glbrntt commented Jul 25, 2019

Is this something we should have tests for?

Ideally, yes. Realistically I think the only thing we can test for here is that the client doesn't become ready before receiving the settings frame and I'm not sure how much value there is in that, WDYT?

@MrMage
Copy link
Collaborator

MrMage commented Jul 25, 2019

Yeah, I wasn't sure about that myself, so let's not test this.

Once you update the conflicts I'll merge :-) And if you do that in the next hour and ping me I can even cut a release ;-)

@MrMage MrMage merged commit c256b4a into grpc:nio Jul 25, 2019
@glbrntt glbrntt deleted the delay-ready branch August 5, 2020 09:06
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.

2 participants