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.

Sign up for GitHub

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

Jump to bottom

Call getaddrinfo from Ruby #139

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+29 −4
Open

Call getaddrinfo from Ruby #139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion contrib/ruby/ext/trilogy-ruby/cext.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ static ID id_socket, id_host, id_port, id_username, id_password, id_found_rows,
id_ivar_affected_rows, id_ivar_fields, id_ivar_last_insert_id, id_ivar_rows, id_ivar_query_time, id_password,
id_database, id_enable_cleartext_plugin, id_ssl_ca, id_ssl_capath, id_ssl_cert, id_ssl_cipher, id_ssl_crl, id_ssl_crlpath, id_ssl_key,
id_ssl_mode, id_tls_ciphersuites, id_tls_min_version, id_tls_max_version, id_multi_statement, id_multi_result,
id_from_code, id_from_errno, id_connection_options, id_max_allowed_packet;
id_from_code, id_from_errno, id_connection_options, id_max_allowed_packet, id_ip_address;

struct trilogy_ctx {
trilogy_conn_t conn;
Expand Down Expand Up @@ -509,6 +509,11 @@ static VALUE rb_trilogy_connect(VALUE self, VALUE encoding, VALUE charset, VALUE
Check_Type(val, T_FIXNUM);
connopt.port = NUM2USHORT(val);
}

if ((val = rb_hash_lookup(opts, ID2SYM(id_ip_address))) != Qnil) {
Check_Type(val, T_STRING);
connopt.ip_address = StringValueCStr(val);
casperisfine marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
connopt.path = (char *)"/tmp/mysql.sock";

Expand Down Expand Up @@ -1215,6 +1220,7 @@ RUBY_FUNC_EXPORTED void Init_cext(void)

id_socket = rb_intern("socket");
id_host = rb_intern("host");
id_ip_address = rb_intern("ip_address");
id_port = rb_intern("port");
id_username = rb_intern("username");
id_password = rb_intern("password");
Expand Down
13 changes: 13 additions & 0 deletions contrib/ruby/lib/trilogy.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,23 @@

class Trilogy
def initialize(options = {})
options = options.dup
options[:port] = options[:port].to_i if options[:port]
mysql_encoding = options[:encoding] || "utf8mb4"
encoding = Trilogy::Encoding.find(mysql_encoding)
charset = Trilogy::Encoding.charset(mysql_encoding)

if options[:host]
addrs = begin
Socket.getaddrinfo(options[:host], options[:port], :PF_UNSPEC, :SOCK_STREAM)
rescue SocketError
raise Trilogy::BaseConnectionError, "Couldn't resolve host: #{options[:host].inspect}"
end

addrs.sort_by!(&:first) # Priority to IPv4
Copy link
Collaborator

Choose a reason for hiding this comment

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

For my own understanding: does prioritizing IPv4 here match what happens when we were calling getaddrinfo directly in trilogy_sock_resolve? Were we resolving to a single address previously, or potentially to multiple? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's a good question to which I don't have a proper answer. This PR is more proof of concept territory because I'm not very good at networking, so there might be a much simpler / better way to do this.

The crux is really that I see processes stuck connecting to MySQL, and I highly suspect this is the cause.

options[:ip_address] = addrs.first[3]
end

@connection_options = options
@connected_host = nil

Expand Down
6 changes: 4 additions & 2 deletions contrib/ruby/test/client_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ def test_trilogy_connection_options
ssl_mode: 4,
tls_min_version: 3,
}
assert_equal expected_connection_options, client.connection_options
actual_options = client.connection_options.dup
actual_options.delete(:ip_address)
assert_equal expected_connection_options, actual_options
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we assert ip_address on the expected connection options instead?

end

def test_trilogy_ping
Expand Down Expand Up @@ -968,7 +970,7 @@ def test_connection_invalid_dns
ex = assert_raises Trilogy::ConnectionError do
new_tcp_client(host: "mysql.invalid", port: 3306)
end
assert_equal "trilogy_connect - unable to connect to mysql.invalid:3306: TRILOGY_DNS_ERROR", ex.message
assert_includes ex.message, %{Couldn't resolve host: "mysql.invalid"}
end

def test_memsize
Expand Down
1 change: 1 addition & 0 deletions inc/trilogy/socket.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ typedef enum {

typedef struct {
char *hostname;
char *ip_address;
char *path;
char *database;
char *username;
Expand Down
5 changes: 4 additions & 1 deletion src/socket.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ static int _cb_raw_close(trilogy_sock_t *_sock)
}

free(sock->base.opts.hostname);
free(sock->base.opts.ip_address);
free(sock->base.opts.path);
free(sock->base.opts.database);
free(sock->base.opts.username);
Expand Down Expand Up @@ -317,6 +318,7 @@ trilogy_sock_t *trilogy_sock_new(const trilogy_sockopt_t *opts)
sock->base.opts = *opts;

sock->base.opts.hostname = strdupnullok(opts->hostname);
sock->base.opts.ip_address = strdupnullok(opts->ip_address);
sock->base.opts.path = strdupnullok(opts->path);
sock->base.opts.database = strdupnullok(opts->database);
sock->base.opts.username = strdupnullok(opts->username);
Expand Down Expand Up @@ -352,7 +354,8 @@ int trilogy_sock_resolve(trilogy_sock_t *_sock)
char port[6];
snprintf(port, sizeof(port), "%hu", sock->base.opts.port);

if (getaddrinfo(sock->base.opts.hostname, port, &hint, &sock->addr) != 0) {
char *address = sock->base.opts.ip_address ? sock->base.opts.ip_address : sock->base.opts.hostname;
if (getaddrinfo(address, port, &hint, &sock->addr) != 0) {
return TRILOGY_DNS_ERR;
}
} else if (sock->base.opts.path != NULL) {
Expand Down
Loading