diff --git a/src/lib/meson.build b/src/lib/meson.build index 4e2981cd..332dbd93 100644 --- a/src/lib/meson.build +++ b/src/lib/meson.build @@ -221,6 +221,7 @@ libxce_test_src = ['testsuite/test.c', 'testsuite/test_netio_bounded_connect.c', 'testsuite/test_netio_batch_dispatch_uaf.c', 'testsuite/test_netio_err_queue.c', + 'testsuite/test_netio_nodelay.c', 'testsuite/test_request_send_propagation.c', 'testsuite/test_raft.c', 'testsuite/test_wal_wire.c', diff --git a/src/lib/netio.c b/src/lib/netio.c index 971e241b..287cc3b3 100644 --- a/src/lib/netio.c +++ b/src/lib/netio.c @@ -2,6 +2,8 @@ #include #include #include +#include +#include #include #include #include @@ -230,6 +232,17 @@ static int set_fd_nonblocking(int fd) return rc; } +/* bug-1890: every netio connection carries small request/reply messages and + * TLS records; with Nagle on, the small record that ends a burst waits for + * the peer's delayed ACK (~40 ms on Linux). Best effort: a socket that + * cannot take the option still works, only slower. */ +static void socket_set_nodelay(int sock) +{ + int on = 1; + + (void)setsockopt(sock, IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on)); +} + static int socket_create(int domain, int type, int port, int backlog) { int sock; @@ -255,6 +268,10 @@ static int socket_create(int domain, int type, int port, int backlog) return -1; } + if ((domain == AF_INET) && (type == SOCK_STREAM)) { + socket_set_nodelay(sock); + } + if (port > 0) { struct sockaddr_in sin; @@ -417,6 +434,7 @@ static int socket_accept(int sock) } fcntl(s, F_SETFD, (fcntl(s, F_GETFD) | FD_CLOEXEC)); + socket_set_nodelay(s); return s; } diff --git a/src/lib/testsuite/test.c b/src/lib/testsuite/test.c index b30c6641..f12510c8 100644 --- a/src/lib/testsuite/test.c +++ b/src/lib/testsuite/test.c @@ -50,6 +50,7 @@ extern void test_netio_close_propagation(void); extern void test_netio_bounded_connect(void); extern void test_netio_batch_dispatch_uaf(void); extern void test_netio_err_queue(void); +extern void test_netio_nodelay(void); extern void test_request_send_propagation(void); extern void test_raft(void); extern void test_wal_wire(void); @@ -178,6 +179,7 @@ int main(int argc, char* argv[]) TEST(test_netio_bounded_connect); TEST(test_netio_batch_dispatch_uaf); TEST(test_netio_err_queue); + TEST(test_netio_nodelay); TEST(test_request_send_propagation); TEST(test_raft); TEST(test_wal_wire); diff --git a/src/lib/testsuite/test_netio_nodelay.c b/src/lib/testsuite/test_netio_nodelay.c new file mode 100644 index 00000000..9c2871f6 --- /dev/null +++ b/src/lib/testsuite/test_netio_nodelay.c @@ -0,0 +1,98 @@ +/* + * test_netio_nodelay.c — bug-1890: every netio TCP connection must run with + * TCP_NODELAY. + * + * With Nagle on, the small TLS record that ends a burst (a packet header, the + * last short chunk of an HA snapshot, a reply) is held back until the peer + * ACKs the data already in flight, and the peer delays that ACK (~40 ms on + * Linux). Observed on 5 of 40 HA snapshot transfers: the primary had written + * every chunk, the standby read the last ones ~40 ms later. + * + * Both ends are covered: outbound sockets (endpoint_init with no port, as + * ha.c/xcectx.c/client.c create them) and inbound ones (endpoint_accept on a + * listening endpoint, as cluster.c's handle_connection does). + */ + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "netio.h" + +static int nodelay_of(int fd) +{ + int on = -1; + socklen_t len = sizeof(on); + + TEST_ASSERT_EQUAL_INT(0, getsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &on, &len)); + return on; +} + +static void test_outbound_socket_has_nodelay(void) +{ + endpoint_t* ep = endpoint_get(); + + TEST_ASSERT_NOT_NULL(ep); + TEST_ASSERT_TRUE(endpoint_init(ep, AF_INET, SOCK_STREAM, 0, 0) >= 0); + + TEST_ASSERT_NOT_EQUAL_MESSAGE(0, nodelay_of(ep->fd), + "bug-1890: outbound netio socket runs with Nagle on"); + + endpoint_unref(ep); + printf("test_outbound_socket_has_nodelay: PASS\n"); +} + +static void test_accepted_socket_has_nodelay(void) +{ + iomgr_t iomgr; + endpoint_t* ls = endpoint_get(); + endpoint_t* ep2 = endpoint_get(); + int cfd; + int port; + int rc = -1; + + TEST_ASSERT_NOT_NULL(ls); + TEST_ASSERT_NOT_NULL(ep2); + + /* endpoint_accept only needs an SSL_CTX to build the session object. */ + memset(&iomgr, 0, sizeof(iomgr)); + iomgr.ssl_ctx = SSL_CTX_new(TLS_client_method()); + TEST_ASSERT_NOT_NULL(iomgr.ssl_ctx); + + for (port = 29870; (port < 29970) && (rc < 0); port++) { + rc = endpoint_init(ls, AF_INET, SOCK_STREAM, port, 4); + } + TEST_ASSERT_TRUE_MESSAGE(rc >= 0, "no free port for the listener"); + port--; + ls->iomgr = &iomgr; + + cfd = socket(AF_INET, SOCK_STREAM, 0); + TEST_ASSERT_TRUE(cfd >= 0); + TEST_ASSERT_EQUAL_INT(0, socket_connect(cfd, "127.0.0.1", port, 1000)); + + TEST_ASSERT_TRUE(endpoint_accept(ls, ep2) >= 0); + TEST_ASSERT_NOT_EQUAL_MESSAGE(0, nodelay_of(ep2->fd), + "bug-1890: accepted netio socket runs with Nagle on"); + + close(cfd); + endpoint_unref(ep2); + endpoint_unref(ls); + SSL_CTX_free(iomgr.ssl_ctx); + printf("test_accepted_socket_has_nodelay: PASS\n"); +} + +void test_netio_nodelay(void) +{ + printf("\n>> netio TCP_NODELAY tests (bug-1890)\n\n"); + + RUN_TEST(test_outbound_socket_has_nodelay); + RUN_TEST(test_accepted_socket_has_nodelay); +}