Support millisecond timeouts for reads, writes, and connects

Summary:
WebScaleSQL Feature: Millisecond Client Timeouts

This diff exposes three new client options:

MYSQL_OPT_CONNECT_TIMEOUT_MS
MYSQL_OPT_READ_TIMEOUT_MS
MYSQL_OPT_WRITE_TIMEOUT_MS

Which are similar to the non-_MS options, except the value is, of
course, in milliseconds.

This diff also changes a number of timeout-related codepaths to use a
structure rather than a naked integer.  This helps prevent many, many
classes of errors that come from accidentally multiplying or dividing by
1000 to convert (or forgetting to), and creates a form of type safety
for timeouts.

Reference patch: b53fcf8

Reviewed By: abal147

Differential Revision: D7298713

fbshipit-source-id: 7719130

Author: lth

client/mysqltest.cc

@@ -6526,6 +6526,7 @@ static void do_connect(struct st_command *command) {
   int con_port = opt_port;
   char *con_options;
   bool con_ssl = 0, con_compress = 0;
+  bool con_timeout_1s = 0, con_timeout_1500ms = 0;
   bool con_pipe = 0, con_shm = 0, con_cleartext_enable = 0;
   struct st_connection *con_slot;
 #if defined(HAVE_OPENSSL)
@@ -6604,13 +6605,20 @@ static void do_connect(struct st_command *command) {
     while (*end && !my_isspace(charset_info, *end)) end++;
 
     size_t con_option_len = end - con_options;
-    char cur_con_option[10];
-    strmake(cur_con_option, con_options, con_option_len);
+    const size_t cur_con_option_len = 32;
+    char cur_con_option[cur_con_option_len + 1];
+    DBUG_ASSERT(cur_con_option_len >= con_option_len);
+    strmake(cur_con_option, con_options,
+            std::min(cur_con_option_len, con_option_len));
 
     if (!std::strcmp(cur_con_option, "SSL"))
       con_ssl = 1;
     else if (!std::strcmp(cur_con_option, "COMPRESS"))
       con_compress = 1;
+    else if (!std::strcmp(cur_con_option, "TIMEOUT_1S"))
+      con_timeout_1s = 1;
+    else if (!std::strcmp(cur_con_option, "TIMEOUT_1500MS"))
+      con_timeout_1500ms = 1;
     else if (!std::strcmp(cur_con_option, "PIPE"))
       con_pipe = 1;
     else if (!std::strcmp(cur_con_option, "SHM"))
@@ -6647,6 +6655,18 @@ static void do_connect(struct st_command *command) {
 
   if (opt_compress || con_compress)
     mysql_options(&con_slot->mysql, MYSQL_OPT_COMPRESS, NullS);
+
+  if (con_timeout_1s) {
+    int timeout = 1;
+    mysql_options(&con_slot->mysql, MYSQL_OPT_READ_TIMEOUT, &timeout);
+    mysql_options(&con_slot->mysql, MYSQL_OPT_WRITE_TIMEOUT, &timeout);
+    mysql_options(&con_slot->mysql, MYSQL_OPT_CONNECT_TIMEOUT, &timeout);
+  } else if (con_timeout_1500ms) {
+    int timeout = 1500;
+    mysql_options(&con_slot->mysql, MYSQL_OPT_READ_TIMEOUT_MS, &timeout);
+    mysql_options(&con_slot->mysql, MYSQL_OPT_WRITE_TIMEOUT_MS, &timeout);
+    mysql_options(&con_slot->mysql, MYSQL_OPT_CONNECT_TIMEOUT_MS, &timeout);
+  }
   mysql_options(&con_slot->mysql, MYSQL_OPT_LOCAL_INFILE, 0);
   mysql_options(&con_slot->mysql, MYSQL_SET_CHARSET_NAME, charset_info->csname);
   if (opt_charsets_dir)

include/mysql.h

@@ -215,7 +215,10 @@ enum mysql_option {
   MYSQL_OPT_SSL_SESSION,
   MYSQL_OPT_SSL_CONTEXT,
   MYSQL_OPT_NET_RECEIVE_BUFFER_SIZE,
-  MYSQL_OPT_COMP_LIB
+  MYSQL_OPT_COMP_LIB,
+  MYSQL_OPT_CONNECT_TIMEOUT_MS,
+  MYSQL_OPT_READ_TIMEOUT_MS,
+  MYSQL_OPT_WRITE_TIMEOUT_MS,
 };
 
 /**
@@ -225,7 +228,7 @@ enum mysql_option {
 struct st_mysql_options_extention;
 
 struct st_mysql_options {
-  unsigned int connect_timeout, read_timeout, write_timeout;
+  timeout_t connect_timeout, read_timeout, write_timeout;
   unsigned int port, protocol;
   unsigned long client_flag;
   char *host, *user, *password, *unix_socket, *db;

include/mysql.h.pp

@@ -107,14 +107,18 @@
 enum mysql_compression_lib { MYSQL_COMPRESSION_ZLIB, MYSQL_COMPRESSION_ZSTD };
 typedef struct ZSTD_CCtx_s ZSTD_CCtx;
 typedef struct ZSTD_DCtx_s ZSTD_DCtx;
+typedef struct {
+  uint value_ms_;
+} timeout_t;
 typedef struct NET {
   struct Vio * vio;
   unsigned char *buff, *buff_end, *write_pos, *read_pos;
   my_socket fd;
   unsigned long remain_in_buf, length, buf_length, where_b;
   unsigned long max_packet, max_packet_size;
   unsigned int pkt_nr, compress_pkt_nr;
-  unsigned int write_timeout, read_timeout, retry_count;
+  timeout_t write_timeout, read_timeout;
+  unsigned int retry_count;
   int fcntl;
   unsigned int *return_status;
   unsigned char reading_or_writing;
@@ -176,9 +180,16 @@
 bool net_write_packet(struct NET *net, const unsigned char *packet,
                       size_t length);
 unsigned long my_net_read(struct NET *net);
-void my_net_set_write_timeout(struct NET *net, unsigned int timeout);
-void my_net_set_read_timeout(struct NET *net, unsigned int timeout);
+void my_net_set_write_timeout(struct NET *net, const timeout_t timeout);
+void my_net_set_read_timeout(struct NET *net, const timeout_t timeout);
 void my_net_set_retry_count(struct NET *net, unsigned int retry_count);
+timeout_t timeout_from_seconds(uint seconds);
+timeout_t timeout_from_millis(uint ms);
+timeout_t timeout_infinite(void);
+bool timeout_is_infinite(const timeout_t t);
+int timeout_is_nonzero(const timeout_t t);
+unsigned int timeout_to_millis(const timeout_t t);
+unsigned int timeout_to_seconds(const timeout_t t);
 struct rand_struct {
   unsigned long seed1, seed2, max_value;
   double max_value_dbl;
@@ -425,11 +436,14 @@
   MYSQL_OPT_SSL_SESSION,
   MYSQL_OPT_SSL_CONTEXT,
   MYSQL_OPT_NET_RECEIVE_BUFFER_SIZE,
-  MYSQL_OPT_COMP_LIB
+  MYSQL_OPT_COMP_LIB,
+  MYSQL_OPT_CONNECT_TIMEOUT_MS,
+  MYSQL_OPT_READ_TIMEOUT_MS,
+  MYSQL_OPT_WRITE_TIMEOUT_MS,
 };
 struct st_mysql_options_extention;
 struct st_mysql_options {
-  unsigned int connect_timeout, read_timeout, write_timeout;
+  timeout_t connect_timeout, read_timeout, write_timeout;
   unsigned int port, protocol;
   unsigned long client_flag;
   char *host, *user, *password, *unix_socket, *db;

include/mysql_com.h

@@ -824,6 +824,17 @@ enum mysql_compression_lib { MYSQL_COMPRESSION_ZLIB, MYSQL_COMPRESSION_ZSTD };
 typedef struct ZSTD_CCtx_s ZSTD_CCtx;
 typedef struct ZSTD_DCtx_s ZSTD_DCtx;
 
+/*
+  In order to avoid confusion about whether a timeout value is in
+  seconds or milliseconds, a timeout_t struct is used.  It simply tracks
+  milliseconds but this helps ensure type safety and clear intention
+  when converting for use in syscalls etc.
+*/
+
+typedef struct {
+  uint value_ms_;
+} timeout_t;
+
 typedef struct NET {
   MYSQL_VIO vio;
   unsigned char *buff, *buff_end, *write_pos, *read_pos;
@@ -836,7 +847,8 @@ typedef struct NET {
   unsigned long remain_in_buf, length, buf_length, where_b;
   unsigned long max_packet, max_packet_size;
   unsigned int pkt_nr, compress_pkt_nr;
-  unsigned int write_timeout, read_timeout, retry_count;
+  timeout_t write_timeout, read_timeout;
+  unsigned int retry_count;
   int fcntl;
   unsigned int *return_status;
   unsigned char reading_or_writing;
@@ -1001,10 +1013,19 @@ bool net_write_command(struct NET *net, unsigned char command,
 bool net_write_packet(struct NET *net, const unsigned char *packet,
                       size_t length);
 unsigned long my_net_read(struct NET *net);
-void my_net_set_write_timeout(struct NET *net, unsigned int timeout);
-void my_net_set_read_timeout(struct NET *net, unsigned int timeout);
+void my_net_set_write_timeout(struct NET *net, const timeout_t timeout);
+void my_net_set_read_timeout(struct NET *net, const timeout_t timeout);
 void my_net_set_retry_count(struct NET *net, unsigned int retry_count);
 
+timeout_t timeout_from_seconds(uint seconds);
+timeout_t timeout_from_millis(uint ms);
+timeout_t timeout_infinite(void);
+bool timeout_is_infinite(const timeout_t t);
+int timeout_is_nonzero(const timeout_t t);
+unsigned int timeout_to_millis(const timeout_t t);
+// toSeconds rounds down.
+unsigned int timeout_to_seconds(const timeout_t t);
+
 struct rand_struct {
   unsigned long seed1, seed2, max_value;
   double max_value_dbl;

include/violite.h

@@ -44,6 +44,7 @@
 #include "mysql/components/services/my_io_bits.h"
 #include "mysql/components/services/my_thread_bits.h"
 #include "mysql/components/services/mysql_socket_bits.h"
+#include "mysql_com.h"
 
 struct Vio;
 
@@ -131,9 +132,11 @@ enum enum_vio_io_event {
   VIO_IO_EVENT_CONNECT
 };
 
-#define VIO_SOCKET_ERROR ((size_t)-1)
-#define VIO_SOCKET_WANT_READ ((size_t)-2)
-#define VIO_SOCKET_WANT_WRITE ((size_t)-3)
+#define VIO_SOCKET_ERROR ((ssize_t)-1)
+#define VIO_SOCKET_WANT_READ ((ssize_t)-2)
+#define VIO_SOCKET_WANT_WRITE ((ssize_t)-3)
+#define VIO_SOCKET_READ_TIMEOUT ((ssize_t)-4)
+#define VIO_SOCKET_WRITE_TIMEOUT ((ssize_t)-5)
 
 #define VIO_LOCALHOST 1            /* a localhost connection */
 #define VIO_BUFFERED_READ 2        /* use buffered read */
@@ -186,16 +189,16 @@ my_socket vio_fd(MYSQL_VIO vio);
 /* Remote peer's address and name in text form */
 bool vio_peer_addr(MYSQL_VIO vio, char *buf, uint16 *port, size_t buflen);
 /* Wait for an I/O event notification. */
-int vio_io_wait(MYSQL_VIO vio, enum enum_vio_io_event event, int timeout);
+int vio_io_wait(MYSQL_VIO vio, enum enum_vio_io_event event, timeout_t timeout);
 bool vio_is_connected(MYSQL_VIO vio);
 #ifndef DBUG_OFF
 ssize_t vio_pending(MYSQL_VIO vio);
 #endif
 /* Set timeout for a network operation. */
-int vio_timeout(MYSQL_VIO vio, uint which, int timeout_sec);
+int vio_timeout(MYSQL_VIO vio, uint which, timeout_t timeout);
 /* Connect to a peer. */
 bool vio_socket_connect(MYSQL_VIO vio, struct sockaddr *addr, socklen_t len,
-                        bool nonblocking, int timeout);
+                        bool nonblocking, timeout_t timeout);
 
 bool vio_get_normalized_ip_string(const struct sockaddr *addr,
                                   size_t addr_length, char *ip_string,
@@ -345,10 +348,10 @@ struct Vio {
   bool localhost = {false};           /* Are we from localhost? */
   enum_vio_type type = {NO_VIO_TYPE}; /* Type of connection */
 
-  int read_timeout = {-1};  /* Timeout value (ms) for read ops. */
-  int write_timeout = {-1}; /* Timeout value (ms) for write ops. */
-  int retry_count = {1};    /* Retry count */
-  bool inactive = {false};  /* Connection has been shutdown */
+  timeout_t read_timeout = {UINT_MAX};  /* Timeout value (ms) for read ops. */
+  timeout_t write_timeout = {UINT_MAX}; /* Timeout value (ms) for write ops. */
+  int retry_count = {1};                /* Retry count */
+  bool inactive = {false};              /* Connection has been shutdown */
 
   struct sockaddr_storage local;  /* Local internet address */
   struct sockaddr_storage remote; /* Remote internet address */
@@ -415,7 +418,7 @@ struct Vio {
   int (*vioshutdown)(MYSQL_VIO) = {nullptr};
   bool (*is_connected)(MYSQL_VIO) = {nullptr};
   bool (*has_data)(MYSQL_VIO) = {nullptr};
-  int (*io_wait)(MYSQL_VIO, enum enum_vio_io_event, int) = {nullptr};
+  int (*io_wait)(MYSQL_VIO, enum enum_vio_io_event, timeout_t) = {nullptr};
   bool (*connect)(MYSQL_VIO, struct sockaddr *, socklen_t, int) = {nullptr};
 #ifdef _WIN32
 #ifdef __clang__

libmysql/libmysql.cc

@@ -946,8 +946,8 @@ void my_net_local_init(NET *net) {
                          &local_net_buffer_length);
 
   net->max_packet = (uint)local_net_buffer_length;
-  my_net_set_read_timeout(net, CLIENT_NET_READ_TIMEOUT);
-  my_net_set_write_timeout(net, CLIENT_NET_WRITE_TIMEOUT);
+  my_net_set_read_timeout(net, timeout_from_seconds(CLIENT_NET_READ_TIMEOUT));
+  my_net_set_write_timeout(net, timeout_from_seconds(CLIENT_NET_WRITE_TIMEOUT));
   my_net_set_retry_count(net, CLIENT_NET_RETRY_COUNT);
   net->max_packet_size =
       MY_MAX(local_net_buffer_length, local_max_allowed_packet);

mysql-test/r/connection_timeout.result

@@ -0,0 +1,20 @@
+select "no timeout, should work";
+no timeout, should work
+no timeout, should work
+select "short timeout, should work", sleep(0.70);
+short timeout, should work	sleep(0.70)
+short timeout, should work	0
+select "long timeout, should fail 1s accuracy", sleep(2);
+ERROR HY000: Read timeout is reached
+select "no timeout, should work";
+no timeout, should work
+no timeout, should work
+select "short timeout, should work", sleep(0.5);
+short timeout, should work	sleep(0.5)
+short timeout, should work	0
+select "short timeout, over one second, should also work", sleep(1.1);
+short timeout, over one second, should also work	sleep(1.1)
+short timeout, over one second, should also work	0
+select "long timeout, should 1500ms accuracy", sleep(3);
+ERROR HY000: Read timeout is reached
+ERROR HY000: Can't connect to MySQL server on '192.0.2.1' (101/110)

mysql-test/t/connection_timeout.test

@@ -0,0 +1,30 @@
+# Two test cases, one for each type of timeout.  First the integer timeout
+# (which uses MYSQL_OPT_READ_TIMEOUT).
+--source include/count_sessions.inc
+--disable_async_client
+connect (con1,127.0.0.1,root,,,,,TIMEOUT_1S);
+connection con1;
+select "no timeout, should work";
+select "short timeout, should work", sleep(0.70);
+--error 2066  # CR_NET_READ_INTERRUPTED
+select "long timeout, should fail 1s accuracy", sleep(2);
+
+# Now confirm MYSQL_OPT_READ_TIMEOUT_MS works, too.
+connect (con2,127.0.0.1,root,,,,,TIMEOUT_1500MS);
+connection con2;
+select "no timeout, should work";
+select "short timeout, should work", sleep(0.5);
+select "short timeout, over one second, should also work", sleep(1.1);
+--error 2066  # CR_NET_READ_INTERRUPTED
+select "long timeout, should 1500ms accuracy", sleep(3);
+
+# 192.0.2.1 is in the TEST-NET range, defined by RFC to never be
+# reachable. Used to confirm connect timeouts.
+--disable_abort_on_error
+--replace_result (101) (101/110) (110) (101/110)
+connect (con3,192.0.2.1,root,,,,,TIMEOUT_1500MS);
+
+--disconnect con1
+--disconnect con2
+connection default;
+--source include/wait_until_count_sessions.inc

plugin/semisync/semisync_master_ack_receiver.cc

@@ -142,7 +142,7 @@ bool Ack_receiver::add_slave(THD *thd) {
   slave.is_leaving = false;
   slave.vio = thd->get_protocol_classic()->get_vio();
   slave.vio->mysql_socket.m_psi = NULL;
-  slave.vio->read_timeout = 1;
+  slave.vio->read_timeout = timeout_from_seconds(1);
 
   /* push_back() may throw an exception */
   try {

plugin/x/client/xconnection_impl.cc

@@ -132,7 +132,7 @@ class Connection_state : public XConnection::State {
     bool res = m_vio->has_data(m_vio);
     if (res) return true;
 
-    return vio_io_wait(m_vio, VIO_IO_EVENT_READ, 0) != 0;
+    return vio_io_wait(m_vio, VIO_IO_EVENT_READ, timeout_from_seconds(0)) != 0;
   }
 
   Vio *m_vio;
@@ -408,10 +408,10 @@ XError Connection_impl::connect(sockaddr *addr, const std::size_t addr_size) {
     return XError(CR_SOCKET_CREATE_ERROR, ER_TEXT_INVALID_SOCKET);
 
   auto vio = vio_new(s, type, 0);
-  auto error =
-      vio_socket_connect(vio, addr, static_cast<socklen_t>(addr_size), false,
-                         details::make_vio_timeout(
-                             m_context->m_connection_config.m_timeout_connect));
+  auto error = vio_socket_connect(
+      vio, addr, static_cast<socklen_t>(addr_size), false,
+      timeout_from_seconds(details::make_vio_timeout(
+          m_context->m_connection_config.m_timeout_connect)));
 
   if (error) {
     err = socket_errno;
@@ -694,7 +694,7 @@ XError Connection_impl::set_read_timeout(const int deadline_seconds) {
                   ER_TEXT_CANT_SET_TIMEOUT_WHEN_NOT_CONNECTED};
   }
 
-  vio_timeout(m_vio, 0, deadline_seconds);
+  vio_timeout(m_vio, 0, timeout_from_seconds(deadline_seconds));
   return {};
 }
 
@@ -704,7 +704,7 @@ XError Connection_impl::set_write_timeout(const int deadline_seconds) {
                   ER_TEXT_CANT_SET_TIMEOUT_WHEN_NOT_CONNECTED};
   }
 
-  vio_timeout(m_vio, 1, deadline_seconds);
+  vio_timeout(m_vio, 1, timeout_from_seconds(deadline_seconds));
   return {};
 }