From c469f6ad9ee46164eed3b33cf067768132ce5e8f Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Tue, 15 Oct 2019 17:21:33 +0300 Subject: [PATCH] Code review minor changes (names, comments). --- src/ae.c | 2 +- src/ae.h | 2 +- src/connection.c | 9 +++++++-- src/connhelpers.h | 25 +++++++++++++++++++++++++ src/rdb.c | 4 ++-- src/server.c | 2 +- 6 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/ae.c b/src/ae.c index 54c0d994e..2c1dae512 100644 --- a/src/ae.c +++ b/src/ae.c @@ -99,7 +99,7 @@ int aeGetSetSize(aeEventLoop *eventLoop) { } /* Tells the next iteration/s of the event processing to set timeout of 0. */ -void aeDontWait(aeEventLoop *eventLoop, int noWait) { +void aeSetDontWait(aeEventLoop *eventLoop, int noWait) { if (noWait) eventLoop->flags |= AE_DONT_WAIT; else diff --git a/src/ae.h b/src/ae.h index bbb43fe6e..9acd72434 100644 --- a/src/ae.h +++ b/src/ae.h @@ -129,6 +129,6 @@ void aeSetBeforeSleepProc(aeEventLoop *eventLoop, aeBeforeSleepProc *beforesleep void aeSetAfterSleepProc(aeEventLoop *eventLoop, aeBeforeSleepProc *aftersleep); int aeGetSetSize(aeEventLoop *eventLoop); int aeResizeSetSize(aeEventLoop *eventLoop, int setsize); -void aeDontWait(aeEventLoop *eventLoop, int noWait); +void aeSetDontWait(aeEventLoop *eventLoop, int noWait); #endif diff --git a/src/connection.c b/src/connection.c index 5a4b48a3e..58d86c31b 100644 --- a/src/connection.c +++ b/src/connection.c @@ -191,6 +191,11 @@ static int connSocketAccept(connection *conn, ConnectionCallbackFunc accept_hand /* Register a write handler, to be called when the connection is writable. * If NULL, the existing handler is removed. + * + * The barrier flag indicates a write barrier is requested, resulting with + * CONN_FLAG_WRITE_BARRIER set. This will ensure that the write handler is + * always called before and not after the read handler in a single event + * loop. */ static int connSocketSetWriteHandler(connection *conn, ConnectionCallbackFunc func, int barrier) { if (func == conn->write_handler) return C_OK; @@ -250,7 +255,7 @@ static void connSocketEventHandler(struct aeEventLoop *el, int fd, void *clientD } /* Normally we execute the readable event first, and the writable - * event laster. This is useful as sometimes we may be able + * event later. This is useful as sometimes we may be able * to serve the reply of a query immediately after processing the * query. * @@ -258,7 +263,7 @@ static void connSocketEventHandler(struct aeEventLoop *el, int fd, void *clientD * asking us to do the reverse: never fire the writable event * after the readable. In such a case, we invert the calls. * This is useful when, for instance, we want to do things - * in the beforeSleep() hook, like fsynching a file to disk, + * in the beforeSleep() hook, like fsync'ing a file to disk, * before replying to a client. */ int invert = conn->flags & CONN_FLAG_WRITE_BARRIER; diff --git a/src/connhelpers.h b/src/connhelpers.h index 2ceccd085..f237c9b1d 100644 --- a/src/connhelpers.h +++ b/src/connhelpers.h @@ -33,10 +33,29 @@ #include "connection.h" +/* These are helper functions that are common to different connection + * implementations (currently sockets in connection.c and TLS in tls.c). + * + * Currently helpers implement the mechanisms for invoking connection + * handlers, tracking in-handler states and dealing with deferred + * destruction (if invoked by a handler). + */ + +/* Called whenever a handler is invoked on a connection and sets the + * CONN_FLAG_IN_HANDLER flag to indicate we're in a handler context. + * + * An attempt to close a connection while CONN_FLAG_IN_HANDLER is + * set will result with deferred close, i.e. setting the CONN_FLAG_CLOSE_SCHEDULED + * instead of destructing it. + */ static inline void enterHandler(connection *conn) { conn->flags |= CONN_FLAG_IN_HANDLER; } +/* Called whenever a handler returns. This unsets the CONN_FLAG_IN_HANDLER + * flag and performs actual close/destruction if a deferred close was + * scheduled by the handler. + */ static inline int exitHandler(connection *conn) { conn->flags &= ~CONN_FLAG_IN_HANDLER; if (conn->flags & CONN_FLAG_CLOSE_SCHEDULED) { @@ -46,6 +65,12 @@ static inline int exitHandler(connection *conn) { return 1; } +/* Helper for connection implementations to call handlers: + * 1. Mark the handler in use. + * 2. Execute the handler (if set). + * 3. Mark the handler as NOT in use and perform deferred close if was + * requested by the handler at any time. + */ static inline int callHandler(connection *conn, ConnectionCallbackFunc handler) { conn->flags |= CONN_FLAG_IN_HANDLER; if (handler) handler(conn); diff --git a/src/rdb.c b/src/rdb.c index 59f22916b..fd279d3b9 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -2365,8 +2365,8 @@ int rdbSaveToSlavesSockets(rdbSaveInfo *rsi) { if (server.rdb_pipe_conns) return C_ERR; /* Before to fork, create a pipe that is used to transfer the rdb bytes to - * the parant, we can't let it write directly to the sockets, since in case - * of TLS we must let the parent handle a contineous TLS state when the + * the parent, we can't let it write directly to the sockets, since in case + * of TLS we must let the parent handle a continuous TLS state when the * child terminates and parent takes over. */ if (pipe(pipefds) == -1) return C_ERR; server.rdb_pipe_read = pipefds[0]; diff --git a/src/server.c b/src/server.c index 3de1347cb..ef3f7f326 100644 --- a/src/server.c +++ b/src/server.c @@ -2051,7 +2051,7 @@ void beforeSleep(struct aeEventLoop *eventLoop) { /* Handle TLS pending data. (must be done before flushAppendOnlyFile) */ tlsProcessPendingData(); /* If tls still has pending unread data don't sleep at all. */ - aeDontWait(server.el, tlsHasPendingData()); + aeSetDontWait(server.el, tlsHasPendingData()); /* Call the Redis Cluster before sleep function. Note that this function * may change the state of Redis Cluster (from ok to fail or vice versa),