perf(client) reduce amount of timers on init_worker (#130)

* perf(client) reduce amount of timers on init_worker

* tests(*) add prove tests for the client and adjust ci for it

* tests(*) reusage of timers, independent on # of workers

* tests(*) simple refactor

* docs(readme) reduce amount of timers on init_worker

* docs(readme) add PR link

Co-authored-by: Vinicius Mignot <[email protected]>

.gitignore

@@ -1,3 +1,3 @@
 .DS_store
 *.rock
-
+t/servroot/

.travis.yml

@@ -25,6 +25,10 @@ install:
   - if [ -z "$OPENRESTY_VER" ]; then export OPENRESTY_VER=1.15.8.3; fi
   - if [ ! -f download-cache/openresty-$OPENRESTY_VER.tar.gz ]; then wget -O download-cache/openresty-$OPENRESTY_VER.tar.gz http://openresty.org/download/openresty-$OPENRESTY_VER.tar.gz; fi
   - if [ ! -f download-cache/luarocks-$LUAROCKS_VER.tar.gz ]; then wget -O download-cache/luarocks-$LUAROCKS_VER.tar.gz https://luarocks.github.io/luarocks/releases/luarocks-$LUAROCKS_VER.tar.gz; fi
+  - if [ ! -f download-cache/cpanm ]; then wget -O download-cache/cpanm https://cpanmin.us/; fi
+  - chmod +x download-cache/cpanm
+  - download-cache/cpanm --notest Test::Nginx >build.log 2>&1 || (cat build.log && exit 1)
+  - download-cache/cpanm --notest --local-lib=$TRAVIS_BUILD_DIR/perl5 local::lib && eval $(perl -I $TRAVIS_BUILD_DIR/perl5/lib/perl5/ -Mlocal::lib)
   - tar -zxf download-cache/openresty-$OPENRESTY_VER.tar.gz
   - tar -zxf download-cache/luarocks-$LUAROCKS_VER.tar.gz
   - pushd openresty-$OPENRESTY_VER
@@ -49,3 +53,4 @@ install:
 script:
   - luacheck ./src
   - busted
+  - TEST_NGINX_RANDOMIZE=1 prove -v -r t

README.md

@@ -50,6 +50,10 @@ Release process:
 4. commit and tag the release
 5. upload rock to LuaRocks
 
+### Unreleased
+
+- Performance: reduce amount of timers on init_worker. [PR 130](https://github.com/Kong/lua-resty-dns-client/pull/130)
+
 ### 6.0.0 (05-Apr-2021)
 
 - BREAKING: the round-robin balancing algorithm is now implemented in

src/resty/dns/client.lua

@@ -33,6 +33,7 @@ local WARN = ngx.WARN
 local DEBUG = ngx.DEBUG
 local PREFIX = "[dns-client] "
 local timer_at = ngx.timer.at
+local get_phase = ngx.get_phase
 
 local math_min = math.min
 local math_max = math.max
@@ -822,6 +823,25 @@ local function syncQuery(qname, r_opts, try_list, count)
     try_status(try_list, "in progress (sync)")
   end
 
+  local supported_semaphore_wait_phases = {
+    rewrite = true,
+    access = true,
+    content = true,
+    timer = true,
+  }
+
+  local ngx_phase = get_phase()
+
+  if not supported_semaphore_wait_phases[ngx_phase] then
+    -- phase not supported by `semaphore:wait`
+    -- return existing query (item)
+    --
+    -- this will avoid:
+    -- "dns lookup pool exceeded retries" (second try and subsequent retries)
+    -- "API disabled in the context of init_worker_by_lua" (first try)
+    return item, nil, try_list
+  end
+
   -- block and wait for the async query to complete
   local ok, err = item.semaphore:wait(poolMaxWait)
   if ok and item.result then

t/00-sanity.t

@@ -0,0 +1,27 @@
+use strict;
+use warnings FATAL => 'all';
+use Test::Nginx::Socket::Lua;
+
+plan tests => 2;
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: load lua-resty-dns-client
+--- config
+    location = /t {
+        access_by_lua_block {
+            local client = require("resty.dns.client")
+            assert(client.init())
+            local host = "localhost"
+            local typ = client.TYPE_A
+            local answers, err = assert(client.resolve(host, { qtype = typ }))
+            ngx.say(answers[1].address)
+        }
+    }
+--- request
+GET /t
+--- response_body
+127.0.0.1
+--- no_error_log

t/01-phases.t

@@ -0,0 +1,66 @@
+use Test::Nginx::Socket;
+
+plan tests => repeat_each() * (blocks() * 5);
+
+workers(6);
+
+no_shuffle();
+run_tests();
+
+__DATA__
+
+=== TEST 1: client supports access phase
+--- config
+    location = /t {
+        access_by_lua_block {
+            local client = require("resty.dns.client")
+            assert(client.init())
+            local host = "localhost"
+            local typ = client.TYPE_A
+            local answers, err = client.resolve(host, { qtype = typ })
+
+            if not answers then
+                ngx.say("failed to resolve: ", err)
+            end
+
+            ngx.say("address name: ", answers[1].name)
+        }
+    }
+--- request
+GET /t
+--- response_body
+address name: localhost
+--- no_error_log
+[error]
+dns lookup pool exceeded retries
+API disabled in the context of init_worker_by_lua
+
+
+
+=== TEST 2: client does not support init_worker phase
+--- http_config eval
+qq {
+    init_worker_by_lua_block {
+        local client = require("resty.dns.client")
+        assert(client.init())
+        local host = "konghq.com"
+        local typ = client.TYPE_A
+        answers, err = client.resolve(host, { qtype = typ })
+    }
+}
+--- config
+    location = /t {
+        access_by_lua_block {
+            ngx.say("answers: ", answers)
+            ngx.say("err: ", err)
+        }
+    }
+--- request
+GET /t
+--- response_body
+answers: nil
+err: dns client error: 101 empty record received
+--- no_error_log
+[error]
+dns lookup pool exceeded retries
+API disabled in the context of init_worker_by_lua

t/02-timer-usage.t

@@ -0,0 +1,78 @@
+use Test::Nginx::Socket;
+
+plan tests => repeat_each() * (blocks() * 5);
+
+workers(6);
+
+no_shuffle();
+run_tests();
+
+__DATA__
+
+=== TEST 1: reuse timers for queries of same name, independent on # of workers
+--- http_config eval
+qq {
+    init_worker_by_lua_block {
+        local client = require("resty.dns.client")
+        assert(client.init({
+            nameservers = { "8.8.8.8" },
+            hosts = {}, -- empty tables to parse to prevent defaulting to /etc/hosts
+            resolvConf = {}, -- and resolv.conf files
+            order = { "A" },
+        }))
+        local host = "httpbin.org"
+        local typ = client.TYPE_A
+        for i = 1, 10 do
+            client.resolve(host, { qtype = typ })
+        end
+
+        local host = "mockbin.org"
+        for i = 1, 10 do
+            client.resolve(host, { qtype = typ })
+        end
+
+        workers = ngx.worker.count()
+        timers = ngx.timer.pending_count()
+    }
+}
+--- config
+    location = /t {
+        access_by_lua_block {
+            local client = require("resty.dns.client")
+            assert(client.init())
+            local host = "httpbin.org"
+            local typ = client.TYPE_A
+            local answers, err = client.resolve(host, { qtype = typ })
+
+            if not answers then
+                ngx.say("failed to resolve: ", err)
+            end
+
+            ngx.say("first address name: ", answers[1].name)
+
+            host = "mockbin.org"
+            answers, err = client.resolve(host, { qtype = typ })
+
+            if not answers then
+                ngx.say("failed to resolve: ", err)
+            end
+
+            ngx.say("second address name: ", answers[1].name)
+
+            ngx.say("workers: ", workers)
+
+            -- should be 2 timers maximum (1 for each hostname)
+            ngx.say("timers: ", timers)
+        }
+    }
+--- request
+GET /t
+--- response_body
+first address name: httpbin.org
+second address name: mockbin.org
+workers: 6
+timers: 2
+--- no_error_log
+[error]
+dns lookup pool exceeded retries
+API disabled in the context of init_worker_by_lua