dlog_logger: commentary for the recent epoll fixes

Some of the recent epoll changes needed an explanation
since it wasn't immediately obvious how they work.

Change-Id: Idecadedcd8bf8a570f65c602d7dbf11e971360f3
Signed-off-by: Michal Bloch <m.bloch@samsung.com>

diff --git a/src/logger/logger.c b/src/logger/logger.c
index c0cca34..7f261e1 100644 (file)
--- a/src/logger/logger.c
+++ b/src/logger/logger.c
@@ -1029,6 +1029,16 @@ static int add_reader(struct logger *server, struct reader *reader)
        assert(server);
 
        if (reader->fd_entity.fd >= 0) {
+               /* Readers who write to file have no FD entity since their FD
+                * is not eligible to be added to epoll. However, epoll primarily
+                * serves to handle pipes getting clogged mid-write (which cannot
+                * happen for files) and is not involved in starting a write,
+                * which happens periodically for all readers anyway regardless
+                * of whether they've been added to epoll or not. The other use
+                * case for epoll - the one where connection FDs leak for unused
+                * buffers - is not a problem here because the deamon is not
+                * supposed to ever stop writing to files. */
+
                int r = add_fd_entity(server, &reader->fd_entity);
                if (r < 0)
                        return r;
@@ -1609,6 +1619,21 @@ static int cond_service_reader(void *ptr, void *user_data)
                return TRUE;
        }
 
+       /* `service_reader()` returns -1 if everything was flushed, or 0 if
+        * a mild error happened that can be recovered from simply by waiting,
+        * the prime example being the pipe getting clogged. As soon as the
+        * reader is available again, we'd like to know about it to ensure
+        * logs are flushed as quickly as possible, which is why the EPOLLOUT.
+        *
+        * On the other hand, we don't want to remove readers from epoll even
+        * if they successfully flushed and have no logs to wait for. Consider
+        * the case where a buffer is unused (for example through libdlog's
+        * buffer disabling feature). If we relied on receiving an error on
+        * calling `write()` to learn that the connection had been closed,
+        * we would never learn about it because there would be no incoming
+        * logs to trigger the flush and so any FDs representing connections
+        * to such buffer would leak until a log finally arrived (which could
+        * be never). This is why waiting is also done on EPOLLHUP. */
        modify_fd_entity(logger, &reader->fd_entity, (r == 0) ? EPOLLOUT : EPOLLHUP);
        return FALSE;
 }