MEDIUM: resolvers: add a ref on server to the used A/AAAA answer item

This patch adds a head list into answer items on servers which use
this record to set their IPs. It makes lookup on duplicated ip faster and
allow to check immediatly if an item is still valid renewing the IP.

This results in better performances on A/AAAA resolutions.

This is an optimization but it could avoid to trigger the haproxy's
internal wathdog in some circumstances. And for this reason
it should be backported as far we can (2.0 ?)

diff --git a/include/haproxy/resolvers-t.h b/include/haproxy/resolvers-t.h
index 96874aabe216019abb28b8bcb6e5c72cf5f4197c..57d85f66ca63e73c9c8f6c2643614c4cf643613a 100644 (file)
--- a/include/haproxy/resolvers-t.h
+++ b/include/haproxy/resolvers-t.h
@@ -115,6 +115,7 @@ struct resolv_answer_item {
        char            target[DNS_MAX_NAME_SIZE+1]; /* Response data: SRV or CNAME type target */
        unsigned int    last_seen;                   /* When was the answer was last seen */
        struct resolv_answer_item *ar_item;          /* pointer to a RRset from the additional section, if exists */
+       struct list     attached_servers;            /* attached server head */
        struct list     list;
 };
 

diff --git a/include/haproxy/resolvers.h b/include/haproxy/resolvers.h
index baf43d5b009aa8d47ff4609fc79a2b7a25edbf56..ba426fa6c345f7b98aee99a9cd821a2a199f40da 100644 (file)
--- a/include/haproxy/resolvers.h
+++ b/include/haproxy/resolvers.h
@@ -41,7 +41,7 @@ int resolv_get_ip_from_response(struct resolv_response *r_res,
                              struct resolv_options *resolv_opts, void *currentip,
                              short currentip_sin_family,
                              void **newip, short *newip_sin_family,
-                             void *owner);
+                             struct server *owner);
 
 void resolv_purge_resolution_answer_records(struct resolv_resolution *resolution);
 int resolv_link_resolution(void *requester, int requester_type, int requester_locked);

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index 714e248725a46a6c4b961d08ad7bc87cad8a5579..1d2a44943018a14da5eb5710b398a31b5712a8b2 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -365,6 +365,7 @@ struct server {
 #endif
 #endif
        struct resolv_srvrq *srvrq;             /* Pointer representing the DNS SRV requeest, if any */
+       struct list ip_rec_item;                /* to attach server to a A or AAAA record item */
        struct {
                const char *file;               /* file where the section appears */
                struct eb32_node id;            /* place in the tree of used IDs */

diff --git a/src/resolvers.c b/src/resolvers.c
index 85b10d6a579f01d94ca71e7b39e7bd4117100ee5..88642403ca4be66f3cb95d01df48bd56a0a788ad 100644 (file)
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -575,7 +575,7 @@ static void resolv_check_response(struct resolv_resolution *res)
        struct resolvers   *resolvers = res->resolvers;
        struct resolv_requester   *req;
        struct resolv_answer_item *item, *itemback;
-       struct server          *srv;
+       struct server          *srv, *srvback;
        struct resolv_srvrq       *srvrq;
 
        list_for_each_entry_safe(item, itemback, &res->response.answer_list, list) {
@@ -592,32 +592,36 @@ static void resolv_check_response(struct resolv_resolution *res)
 
                /* Remove obsolete items */
                if (tick_is_lt(tick_add(item->last_seen, resolvers->hold.obsolete), now_ms)) {
-                       if (item->type != DNS_RTYPE_SRV)
-                               goto rm_obselete_item;
-
-                       list_for_each_entry(req, &res->requesters, list) {
-                               if ((srvrq = objt_resolv_srvrq(req->owner)) == NULL)
-                                       continue;
-
+                       if (item->type == DNS_RTYPE_A || item->type == DNS_RTYPE_AAAA) {
                                /* Remove any associated server */
-                               for (srv = srvrq->proxy->srv; srv != NULL; srv = srv->next) {
-                                       HA_SPIN_LOCK(SERVER_LOCK, &srv->lock);
-                                       if (srv->srvrq == srvrq && srv->svc_port == item->port &&
-                                           item->data_len == srv->hostname_dn_len &&
-                                           !resolv_hostname_cmp(srv->hostname_dn, item->target, item->data_len)) {
-                                               resolv_unlink_resolution(srv->resolv_requester, 0);
-                                               srvrq_update_srv_status(srv, 1);
-                                               ha_free(&srv->hostname);
-                                               ha_free(&srv->hostname_dn);
-                                               srv->hostname_dn_len = 0;
-                                               memset(&srv->addr, 0, sizeof(srv->addr));
-                                               srv->svc_port = 0;
+                               list_for_each_entry_safe(srv, srvback, &item->attached_servers, ip_rec_item) {
+                                       LIST_DEL_INIT(&srv->ip_rec_item);
+                               }
+                       }
+                       else if (item->type == DNS_RTYPE_SRV) {
+                               list_for_each_entry(req, &res->requesters, list) {
+                                       if ((srvrq = objt_resolv_srvrq(req->owner)) == NULL)
+                                               continue;
+
+                                       /* Remove any associated server */
+                                       for (srv = srvrq->proxy->srv; srv != NULL; srv = srv->next) {
+                                               HA_SPIN_LOCK(SERVER_LOCK, &srv->lock);
+                                               if (srv->srvrq == srvrq && srv->svc_port == item->port &&
+                                                   item->data_len == srv->hostname_dn_len &&
+                                                   !resolv_hostname_cmp(srv->hostname_dn, item->target, item->data_len)) {
+                                                       resolv_unlink_resolution(srv->resolv_requester, 0);
+                                                       srvrq_update_srv_status(srv, 1);
+                                                       ha_free(&srv->hostname);
+                                                       ha_free(&srv->hostname_dn);
+                                                       srv->hostname_dn_len = 0;
+                                                       memset(&srv->addr, 0, sizeof(srv->addr));
+                                                       srv->svc_port = 0;
+                                               }
+                                               HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
                                        }
-                                       HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
                                }
                        }
 
-                 rm_obselete_item:
                        LIST_DELETE(&item->list);
                        if (item->ar_item) {
                                pool_free(resolv_answer_item_pool, item->ar_item);
@@ -902,6 +906,7 @@ static int resolv_validate_dns_response(unsigned char *resp, unsigned char *bufe
                /* initialization */
                answer_record->ar_item = NULL;
                answer_record->last_seen = TICK_ETERNITY;
+               LIST_INIT(&answer_record->attached_servers);
 
                offset = 0;
                len = resolv_read_name(resp, bufend, reader, tmpname, DNS_MAX_NAME_SIZE, &offset, 0);
@@ -1153,6 +1158,7 @@ static int resolv_validate_dns_response(unsigned char *resp, unsigned char *bufe
                if (answer_record == NULL)
                        goto invalid_resp;
                answer_record->last_seen = TICK_ETERNITY;
+               LIST_INIT(&answer_record->attached_servers);
 
                offset = 0;
                len = resolv_read_name(resp, bufend, reader, tmpname, DNS_MAX_NAME_SIZE, &offset, 0);
@@ -1311,7 +1317,6 @@ static int resolv_validate_dns_response(unsigned char *resp, unsigned char *bufe
 
        /* Save the number of records we really own */
        r_res->header.arcount = nb_saved_records;
-
        resolv_check_response(resolution);
        return RSLV_RESP_VALID;
 
@@ -1335,9 +1340,9 @@ int resolv_get_ip_from_response(struct resolv_response *r_res,
                              struct resolv_options *resolv_opts, void *currentip,
                              short currentip_sin_family,
                              void **newip, short *newip_sin_family,
-                             void *owner)
+                             struct server *owner)
 {
-       struct resolv_answer_item *record;
+       struct resolv_answer_item *record, *found_record = NULL;
        int family_priority;
        int currentip_found;
        unsigned char *newip4, *newip6;
@@ -1346,6 +1351,10 @@ int resolv_get_ip_from_response(struct resolv_response *r_res,
        int score, max_score;
        int allowed_duplicated_ip;
 
+       /* srv is linked to an alive ip record */
+       if (owner && LIST_INLIST(&owner->ip_rec_item))
+               return RSLV_UPD_NO;
+
        family_priority   = resolv_opts->family_prio;
        allowed_duplicated_ip = resolv_opts->accept_duplicate_ip;
        *newip = newip4   = newip6 = NULL;
@@ -1410,9 +1419,25 @@ int resolv_get_ip_from_response(struct resolv_response *r_res,
                /* Check if the IP found in the record is already affected to a
                 * member of a group.  If not, the score should be incremented
                 * by 2. */
-               if (owner && snr_check_ip_callback(owner, ip, &ip_type)) {
-                       if (!allowed_duplicated_ip) {
-                               continue;
+               if (owner) {
+                       struct server *srv;
+                       int already_used = 0;
+
+                       list_for_each_entry(srv, &record->attached_servers, ip_rec_item) {
+                               if (srv == owner)
+                                       continue;
+                               if (srv->proxy == owner->proxy) {
+                                       already_used = 1;
+                                       break;
+                               }
+                       }
+                       if (already_used) {
+                               if (!allowed_duplicated_ip) {
+                                       continue;
+                               }
+                       }
+                       else {
+                               score += 2;
                        }
                } else {
                        score += 2;
@@ -1439,9 +1464,15 @@ int resolv_get_ip_from_response(struct resolv_response *r_res,
                                newip4 = ip;
                        else
                                newip6 = ip;
+                       found_record = record;
                        currentip_found = currentip_sel;
-                       if (score == 15)
+                       if (score == 15) {
+                               /* this was not registered on the current record but it matches
+                                * let's fix it (it may comes from state file */
+                               if (owner)
+                                       LIST_APPEND(&found_record->attached_servers, &owner->ip_rec_item);
                                return RSLV_UPD_NO;
+                       }
                        max_score = score;
                }
        } /* list for each record entries */
@@ -1494,6 +1525,12 @@ int resolv_get_ip_from_response(struct resolv_response *r_res,
        return RSLV_UPD_NO;
 
  not_found:
+
+       /* the ip of this record was chosen for the server */
+       if (owner && found_record) {
+               LIST_APPEND(&found_record->attached_servers, &owner->ip_rec_item);
+       }
+
        list_for_each_entry(record, &r_res->answer_list, list) {
                /* Move the first record to the end of the list, for internal
                 * round robin */
@@ -1832,12 +1869,18 @@ void resolv_unlink_resolution(struct resolv_requester *requester, int safe)
 {
        struct resolv_resolution *res;
        struct resolv_requester  *req;
+       struct server *srv;
 
        /* Nothing to do */
        if (!requester || !requester->resolution)
                return;
        res = requester->resolution;
 
+       /* remove ref from the resolution answer item list to the requester */
+       if ((srv = objt_server(requester->owner)) != NULL) {
+               LIST_DEL_INIT(&srv->ip_rec_item);
+       }
+
        /* Clean up the requester */
        LIST_DELETE(&requester->list);
        requester->resolution = NULL;

diff --git a/src/server.c b/src/server.c
index c2c52e7a01ab92486bde594e8c10a7daa972e23a..4a7044b4f2e8e69306c042243291d90e1203b2e4 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -2160,6 +2160,7 @@ struct server *new_server(struct proxy *proxy)
        srv->proxy = proxy;
        srv->pendconns = EB_ROOT;
        LIST_APPEND(&servers_list, &srv->global_list);
+       LIST_INIT(&srv->ip_rec_item);
 
        srv->next_state = SRV_ST_RUNNING; /* early server setup */
        srv->last_change = now.tv_sec;
@@ -3525,6 +3526,7 @@ int snr_resolution_error_cb(struct resolv_requester *requester, int error_code)
        if (!snr_update_srv_status(s, 1)) {
                memset(&s->addr, 0, sizeof(s->addr));
                HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock);
+               LIST_DEL_INIT(&s->ip_rec_item);
                return 0;
        }
        HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock);