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

Establish socket connection in Ruby #218

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

jhawthorn
Copy link
Member

@jhawthorn jhawthorn commented Jan 31, 2025

Inspired by @byroot's work in #139

This moves the responsibility for establishing the initial TCP or unix socket connection to Ruby. This allows us to lean on Ruby's implementation and configuration of DNS timeouts as well as happy eyeballs/fast fallback.

In addition to improved timeouts, this should also deliver more detailed and specific error messages.

Before

❯ be ruby connect_benchmark.rb
Warming up --------------------------------------
trilogy connect/close
                       761.000  i/100ms
Calculating -------------------------------------
trilogy connect/close
                          8.238k (± 2.5%) i/s -     41.855k in   5.083964s

After

Warming up --------------------------------------
trilogy connect/close
                       777.000  i/100ms
Calculating -------------------------------------
trilogy connect/close
                          8.169k (± 3.6%) i/s -     41.181k in   5.047361s

Doesn't seem to be a significant performance difference

@jhawthorn jhawthorn mentioned this pull request Feb 5, 2025
This moves the responsability for establishing the initial TCP or unix
socket connection to Ruby. This allows us to lean on Ruby's
implementation and configuration of DNS timeouts as well as happy
eyeballs/fast fallback.

In addition to improved timeouts, this should also deliver more detailed
and specific error messages.

Co-authored-by: Adam Hess <[email protected]>
Co-authored-by: Jean Boussier <[email protected]>
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