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

feat(s2n-quic-transport): Support de-duplicating concurrent connections to the same peer #2272

Merged
merged 3 commits into from
Jul 17, 2024

Conversation

Mark-Simulacrum
Copy link
Collaborator

Resolved issues:

resolves #2211

Description of changes:

This implements support for utilizing s2n-quic to track open (or pending open) connection handles, effectively de-duplicating requests to open a connection.

Call-outs:

  • Currently using a HashMap<Connect, InternalId>. It's possible this could be made intrusive and save memory, though I'm not sure if the SNI and initial open SocketAddr are actually tracked within the connection state. This is purely an optimization though and shouldn't affect public API if we choose to apply it.
  • I'm not very confident that all of the structures/counters are updated in the right places. For example, I had to add a manual drop of the tracking state rather than relying on implicitly dropping it. There's nothing in that code that drops other maps though, so that makes me suspect it's the wrong approach.
  • Locally I seem to have also managed to break the optimistic ack test (5 -> 4 skipped packets). However I don't see any of my code as doing anything without explicit enablement, so not sure yet what exactly is going on there. Opening for review regardless while I chase that down.

Testing:

  • Added a basic test that shows de-duplication works.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@WesleyRosenblum WesleyRosenblum changed the title Support de-duplicating concurrent connections to the same peer feat(s2n-quic-transport): Support de-duplicating concurrent connections to the same peer Jul 16, 2024
@Mark-Simulacrum Mark-Simulacrum force-pushed the dedup-connections branch 2 times, most recently from 0c9e33b to 8aa2274 Compare July 17, 2024 14:53
Open connections can now be found when calling connect(), including
in-flight connections. Currently this property is applied to *all*
connections, the next commit will add support for configuring whether
the application is interested in this support.
(No longer actually changed by previous commits, but seems worth the
comment regardless to save future time).
@camshaft camshaft enabled auto-merge (squash) July 17, 2024 19:20
@camshaft camshaft merged commit d55d258 into aws:main Jul 17, 2024
120 of 121 checks passed
@Mark-Simulacrum Mark-Simulacrum deleted the dedup-connections branch July 17, 2024 19:51
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.

Allow for deduplicating outgoing connection attempts
2 participants