lib/cache: simplify logic for qry->flags.CACHE_TRIED

As a side effect, this solves an issue which could be seen
during resolver startup where the trust anchor update would fail.
[taupd ] active refresh failed for . with rcode: 2

The cause is that for queries started with .flags.NO_CACHE,
we'd skip the section setting .flags.CACHE_TRIED, and consequently
kr_rule_local_data_answer() would get run more often than expected.

The new logic should be also much simpler to follow.
We always apply cache (and policy) just once per kr_query.

diff --git a/lib/cache/api.c b/lib/cache/api.c
index 046dae20335edafa990c7d0c115c2cd524358f5f..d542060c9e371539a0fefc5b341a81f9b5866006 100644 (file)
--- a/lib/cache/api.c
+++ b/lib/cache/api.c
@@ -314,32 +314,31 @@ int cache_peek(kr_layer_t *ctx, knot_pkt_t *pkt)
        struct kr_request *req = ctx->req;
        struct kr_query *qry = req->current_query;
 
-       /* TODO: review when to run this?  We want to process rules here
-        * even when some of the cache exit-conditions happen.  NO_CACHE in particular. */
-       if (!req->options.PASSTHRU_LEGACY && !qry->flags.CACHE_TRIED) {
+       if (ctx->state & (KR_STATE_FAIL|KR_STATE_DONE))
+               return ctx->state;
+       /* ATM cache only peeks for qry->sname and that would be useless
+        * to repeat on every iteration, so disable it from now on.
+        * Note that xNAME causes a followup kr_query, so cache will get re-tried.
+        * LATER(optim.): assist with more precise QNAME minimization. */
+       if (qry->flags.CACHE_TRIED && !qry->stale_cb)
+               return ctx->state;
+       qry->flags.CACHE_TRIED = true;
+
+       if (!req->options.PASSTHRU_LEGACY) {
                int ret = kr_rule_local_data_answer(qry, pkt);
                if (ret < 0)
                        ctx->state = KR_STATE_FAIL;
-               if (ret != 0) {
-                       qry->flags.CACHE_TRIED = true;
+               if (ret != 0)
                        return ctx->state;
-               }
        }
 
        /* We first check various exit-conditions and then call the _real function. */
-
        if (!kr_cache_is_open(&req->ctx->cache)
-           || ctx->state & (KR_STATE_FAIL|KR_STATE_DONE) || qry->flags.NO_CACHE
-           || (qry->flags.CACHE_TRIED && !qry->stale_cb)
+           || qry->flags.NO_CACHE
            || !check_rrtype(qry->stype, qry) /* LATER: some other behavior for some of these? */
            || qry->sclass != KNOT_CLASS_IN) {
                return ctx->state; /* Already resolved/failed or already tried, etc. */
        }
-       /* ATM cache only peeks for qry->sname and that would be useless
-        * to repeat on every iteration, so disable it from now on.
-        * Note that xNAME causes a followup kr_query, so cache will get re-tried.
-        * LATER(optim.): assist with more precise QNAME minimization. */
-       qry->flags.CACHE_TRIED = true;
 
        if (qry->stype == KNOT_RRTYPE_NSEC) {
                VERBOSE_MSG(qry, "=> skipping stype NSEC\n");