resolved: handle properly if there are multiple transactions for the same key per...

When the zone probing code looks for a transaction to reuse it will
refuse to look at transactions that have been answered from cache or the
zone itself, but insist on the network. This has the effect that there
might be multiple transactions around for the same key on the same
scope. Previously we'd track all transactions in a hashmap, indexed by
the key, which implied that there would be only one transaction per key,
per scope. With this change the hashmap will only store the most recent
transaction per key, and a linked list will be used to track all
transactions per scope, allowing multiple per-key per-scope.

Note that the linked list fields for this actually already existed in
the DnsTransaction structure, but were previously unused.

diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c
index 6b496af41212c2bb8dd53dac277fc7b43f07bc0f..fc4ae57ce0805149bacbb80bfccecdd9542d5726 100644 (file)
--- a/src/resolve/resolved-dns-scope.c
+++ b/src/resolve/resolved-dns-scope.c
@@ -70,11 +70,11 @@ int dns_scope_new(Manager *m, DnsScope **ret, Link *l, DnsProtocol protocol, int
 }
 
 static void dns_scope_abort_transactions(DnsScope *s) {
-        DnsTransaction *t;
-
         assert(s);
 
-        while ((t = hashmap_first(s->transactions))) {
+        while (s->transactions) {
+                DnsTransaction *t = s->transactions;
+
                 /* Abort the transaction, but make sure it is not
                  * freed while we still look at it */
 
@@ -100,7 +100,7 @@ DnsScope* dns_scope_free(DnsScope *s) {
         while (s->query_candidates)
                 dns_query_candidate_free(s->query_candidates);
 
-        hashmap_free(s->transactions);
+        hashmap_free(s->transactions_by_key);
 
         while ((rr = ordered_hashmap_steal_first(s->conflict_queue)))
                 dns_resource_record_unref(rr);
@@ -653,7 +653,7 @@ DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsResourceKey *key,
 
         /* Try to find an ongoing transaction that is a equal to the
          * specified question */
-        t = hashmap_get(scope->transactions, key);
+        t = hashmap_get(scope->transactions_by_key, key);
         if (!t)
                 return NULL;
 

diff --git a/src/resolve/resolved-dns-scope.h b/src/resolve/resolved-dns-scope.h
index 32e6961757808e6fbbc4075ae895f193229a02b4..7876410b7d6be7cbcd7449efefbac16bc0b7645c 100644 (file)
--- a/src/resolve/resolved-dns-scope.h
+++ b/src/resolve/resolved-dns-scope.h
@@ -58,9 +58,19 @@ struct DnsScope {
         usec_t resend_timeout;
         usec_t max_rtt;
 
-        Hashmap *transactions;
         LIST_HEAD(DnsQueryCandidate, query_candidates);
 
+        /* Note that we keep track of ongoing transactions in two
+         * ways: once in a hashmap, indexed by the rr key, and once in
+         * a linked list. We use the hashmap to quickly find
+         * transactions we can reuse for a key. But note that there
+         * might be multiple transactions for the same key (because
+         * the zone probing can't reuse a transaction answered from
+         * the zone or the cache), and the hashmap only tracks the
+         * most recent entry. */
+        Hashmap *transactions_by_key;
+        LIST_HEAD(DnsTransaction, transactions);
+
         LIST_FIELDS(DnsScope, scopes);
 };
 

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 519358ffb12bd74fbadb7284ed1ec34ce888ffc4..c65c9c9f492160772f6a834a09b94c49414e4694 100644 (file)
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -49,7 +49,8 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) {
         dns_stream_free(t->stream);
 
         if (t->scope) {
-                hashmap_remove(t->scope->transactions, t->key);
+                hashmap_remove_value(t->scope->transactions_by_key, t->key, t);
+                LIST_REMOVE(transactions_by_scope, t->scope->transactions, t);
 
                 if (t->id != 0)
                         hashmap_remove(t->scope->manager->dns_transactions, UINT_TO_PTR(t->id));
@@ -94,7 +95,7 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key)
         if (r < 0)
                 return r;
 
-        r = hashmap_ensure_allocated(&s->transactions, &dns_resource_key_hash_ops);
+        r = hashmap_ensure_allocated(&s->transactions_by_key, &dns_resource_key_hash_ops);
         if (r < 0)
                 return r;
 
@@ -118,12 +119,13 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key)
                 return r;
         }
 
-        r = hashmap_put(s->transactions, t->key, t);
+        r = hashmap_replace(s->transactions_by_key, t->key, t);
         if (r < 0) {
                 hashmap_remove(s->manager->dns_transactions, UINT_TO_PTR(t->id));
                 return r;
         }
 
+        LIST_PREPEND(transactions_by_scope, s->transactions, t);
         t->scope = s;
 
         if (ret)