Compatibility with fiber scheduler. about trilogy HOT 7 OPEN

I'm not sure where this is used, but it might be better to use the rb_wait_* functions too:

https://github.com/github/trilogy/blob/2dd91e5d1974b29473ad646e5dba0a50562fe37a/src/socket.c#L50

_cb_wait is not used at all in the Ruby gem. That's the default for anyone using the C lib directly, but for Ruby we swap it out at https://github.com/github/trilogy/blob/2dd91e5d1974b29473ad646e5dba0a50562fe37a/contrib/ruby/ext/trilogy-ruby/cext.c#L251-L253 with a wait_cb using rb_wait_for_single_fd.

That's about the extent of my knowledge at the moment. I'd happy to help make whatever changes are desirable here, but I'll probably need to learn a lot more about the fiber scheduler, the difference between rb_wait_for_single_fd and rb_io_wait, etc.

from trilogy.

I am happy to make a PR if you are willing to work with me getting it merged?

from trilogy.

Hmm I found https://github.com/github/trilogy/blob/2dd91e5d1974b29473ad646e5dba0a50562fe37a/contrib/ruby/ext/trilogy-ruby/cext.c#L214

Would be more efficient if we could use rb_io_wait but this is a good start.

from trilogy.

I'm not sure where this is used, but it might be better to use the rb_wait_* functions too:

https://github.com/github/trilogy/blob/2dd91e5d1974b29473ad646e5dba0a50562fe37a/src/socket.c#L50

from trilogy.

I am happy to make a PR if you are willing to work with me getting it merged?

Yes, I can do that!

from trilogy.

I started playing around with the code.

static int _cb_ruby_wait(trilogy_sock_t *sock, trilogy_wait_t wait)
{
    struct timeval *timeout = NULL;
    int wait_flag = 0;

    switch (wait) {
    case TRILOGY_WAIT_READ:
        timeout = &sock->opts.read_timeout;
        wait_flag = RB_WAITFD_IN;
        break;

    case TRILOGY_WAIT_WRITE:
        timeout = &sock->opts.write_timeout;
        wait_flag = RB_WAITFD_OUT;
        break;

    case TRILOGY_WAIT_HANDSHAKE:
        timeout = &sock->opts.connect_timeout;
        wait_flag = RB_WAITFD_IN;
        break;

    default:
        return TRILOGY_ERR;
    }

    if (timeout->tv_sec == 0 && timeout->tv_usec == 0) {
        timeout = NULL;
    }

    int fd = trilogy_sock_fd(sock);
    if (rb_wait_for_single_fd(fd, wait_flag, timeout) <= 0)
        return TRILOGY_SYSERR;

    return 0;
}

The rb_wait_for_single_fd should be replaced with rb_io_wait(io, RB_INT2NUM(wait_flag), rb_fiber_scheduler_make_timeout(timeout)).

The io object can be created by using RB_IO_OPEN but I'll need to check the exact syntax.

I think the callback argument will need to be added or the existing context should be changed to store the io, e.g.

struct trilogy_ctx {
    trilogy_conn_t conn;
    VALUE io;

    char server_version[TRILOGY_SERVER_VERSION_SIZE + 1];
    unsigned int query_flags;
};

Do you have any thoughts about how to best achieve this? Maybe we can add a void *context to struct trilogy_sock_t which refers to trilogy_ctx.

Also, just for your reference, naming conventions with trailing _t is highly discouraged by POSIX standard. https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xsh_chap02.html#tag_22_02_12 - _t suffix is reserved for system types.

from trilogy.

I've opened a draft PR with the above changes.

from trilogy.