daemon: more avoidance of excessive TCP reconnections

Previously this penalization was only triggered if the remote server
closed TCP.  Now it's extended to us closing it when the server
(only) sends back some nonsense.  At least for the cases which I could
see immediately.

That's just three trivial one-line additions; the rest is refactoring.

diff --git a/daemon/io.c b/daemon/io.c
index 48bfed301554a1ddc2e93f11c09df315447ad7b5..ac0d96928d4aa73b3667ca486ed0d7b138a4fba0 100644 (file)
--- a/daemon/io.c
+++ b/daemon/io.c
@@ -337,33 +337,6 @@ void tcp_timeout_trigger(uv_timer_t *timer)
        }
 }
 
-static void tcp_disconnect(struct session *s, int errcode)
-{
-       if (kr_log_is_debug(IO, NULL)) {
-               struct sockaddr *peer = session_get_peer(s);
-               char *peer_str = kr_straddr(peer);
-               kr_log_debug(IO, "=> connection to '%s' closed by peer (%s)\n",
-                              peer_str ? peer_str : "",
-                              uv_strerror(errcode));
-       }
-
-       if (!session_was_useful(s) && session_flags(s)->outgoing) {
-               /* We want to penalize the IP address, if a task is asking a query.
-                * It might not be the right task, but that doesn't matter so much
-                * for attributing the useless session to the IP address. */
-               struct qr_task *t = session_tasklist_get_first(s);
-               struct kr_query *qry = NULL;
-               if (t) {
-                       struct kr_request *req = worker_task_request(t);
-                       qry = array_tail(req->rplan.pending);
-               }
-               if (qry) /* We reuse the error for connection, as it's quite similar. */
-                       qry->server_selection.error(qry, worker_task_get_transport(t),
-                                                       KR_SELECTION_TCP_CONNECT_FAILED);
-       }
-       worker_end_tcp(s);
-}
-
 static void tcp_recv(uv_stream_t *handle, ssize_t nread, const uv_buf_t *buf)
 {
        struct session *s = handle->data;
@@ -381,7 +354,16 @@ static void tcp_recv(uv_stream_t *handle, ssize_t nread, const uv_buf_t *buf)
        }
 
        if (nread < 0 || !buf->base) {
-               tcp_disconnect(s, nread);
+               if (kr_log_is_debug(IO, NULL)) {
+                       struct sockaddr *peer = session_get_peer(s);
+                       char *peer_str = kr_straddr(peer);
+                       kr_log_debug(IO, "=> connection to '%s' closed by peer (%s)\n",
+                                      peer_str ? peer_str : "",
+                                      uv_strerror(nread));
+               }
+
+               session_tcp_penalize(s);
+               worker_end_tcp(s);
                return;
        }
 

diff --git a/daemon/session.c b/daemon/session.c
index a1f22072b0ccf89a01262ab397d6abbf507358c1..ed0ff6869472cb37eb58a4c12852e01c44ea0c57 100644 (file)
--- a/daemon/session.c
+++ b/daemon/session.c
@@ -123,11 +123,6 @@ void session_close(struct session *session)
        }
 }
 
-bool session_was_useful(const struct session *session)
-{
-       return session->was_useful;
-}
-
 int session_start_read(struct session *session)
 {
        return io_start_read(session->handle);
@@ -582,6 +577,24 @@ ssize_t session_wirebuf_trim(struct session *session, ssize_t len)
        return len;
 }
 
+void session_tcp_penalize(struct session *s)
+{
+       if (s->was_useful || !s->sflags.outgoing)
+               return;
+       /* We want to penalize the IP address, if a task is asking a query.
+        * It might not be the right task, but that doesn't matter so much
+        * for attributing the useless session to the IP address. */
+       struct qr_task *t = session_tasklist_get_first(s);
+       struct kr_query *qry = NULL;
+       if (t) {
+               struct kr_request *req = worker_task_request(t);
+               qry = array_tail(req->rplan.pending);
+       }
+       if (qry) /* We reuse the error for connection, as it's quite similar. */
+               qry->server_selection.error(qry, worker_task_get_transport(t),
+                                               KR_SELECTION_TCP_CONNECT_FAILED);
+}
+
 knot_pkt_t *session_produce_packet(struct session *session, knot_mm_t *mm)
 {
        session->sflags.wirebuf_error = false;
@@ -617,6 +630,7 @@ knot_pkt_t *session_produce_packet(struct session *session, knot_mm_t *mm)
                msg_size = knot_wire_read_u16(msg_start);
                if (msg_size >= session->wire_buf_size) {
                        session->sflags.wirebuf_error = true;
+                       session_tcp_penalize(session);
                        return NULL;
                }
                if (msg_size + 2 > wirebuf_msg_data_size) {
@@ -624,6 +638,7 @@ knot_pkt_t *session_produce_packet(struct session *session, knot_mm_t *mm)
                }
                if (msg_size == 0) {
                        session->sflags.wirebuf_error = true;
+                       session_tcp_penalize(session);
                        return NULL;
                }
                msg_start += 2;
@@ -631,6 +646,7 @@ knot_pkt_t *session_produce_packet(struct session *session, knot_mm_t *mm)
                msg_size = wirebuf_msg_data_size;
        } else {
                session->sflags.wirebuf_error = true;
+               session_tcp_penalize(session);
                return NULL;
        }
 

diff --git a/daemon/session.h b/daemon/session.h
index 603d7cb450f2cc72f90839d9ad26e4570fb2890e..1f95ac5b6888ea61c214f373b9894fe76aaf82cd 100644 (file)
--- a/daemon/session.h
+++ b/daemon/session.h
@@ -91,8 +91,9 @@ int session_tasklist_finalize_expired(struct session *session);
 /** Both of task lists (associated & waiting). */
 /** Check if empty. */
 bool session_is_empty(const struct session *session);
-/** Return whether session seems to have done something useful. */
-bool session_was_useful(const struct session *session);
+/** Penalize this server if the session hasn't been useful (and is outgoing). */
+void session_tcp_penalize(struct session *session);
+
 /** Get pointer to session flags */
 struct session_flags *session_flags(struct session *session);
 /** Get pointer to peer address. */