[GitHub] [apisix] tokers commented on a change in pull request #3686: feat: support SRV record

[GitBox <gi...@apache.org>]

tokers commented on a change in pull request #3686:
URL: https://github.com/apache/apisix/pull/3686#discussion_r584012410



##########
File path: apisix/core/dns/client.lua
##########
@@ -29,6 +31,67 @@ local _M = {
 }
 
 
+local function gcd(a, b)
+    if b == 0 then
+        return a
+    end
+
+    return gcd(b, a % b)
+end
+
+
+local function resolve_srv(client, answers)
+    if #answers == 0 then
+        return nil, "empty SRV record"
+    end
+
+    local resolved_answers = {}
+    local answer_to_count = {}
+    for _, answer in ipairs(answers) do
+        if answer.type ~= client.TYPE_SRV then
+            return nil, "mess SRV with other record"
+        end
+
+        local resolved, err = client.resolve(answer.target)
+        if not resolved then
+            local msg = "failed to resolve SRV record " .. answer.target .. ": " .. err
+            return nil, msg
+        end
+
+        log.info("dns resolve SRV ", answer.target, ", result: ",
+                 json.delay_encode(resolved))
+
+        local weight = answer.weight
+        if weight == 0 then
+            weight = 1
+        end
+
+        local count = #resolved
+        answer_to_count[answer] = count
+        -- one target may have multiple resolved results
+        for _, res in ipairs(resolved) do
+            local copy = table.deepcopy(res)
+            copy.weight = weight / count
+            copy.port = answer.port
+            insert_tab(resolved_answers, copy)
+        end
+    end
+
+    -- find the least common multiple of the counts
+    local lcm = answer_to_count[answers[1]]
+    for i = 1, #answers do

Review comment:
       We can iterate from `i = 2`.

##########
File path: apisix/core/dns/client.lua
##########
@@ -29,6 +31,67 @@ local _M = {
 }
 
 
+local function gcd(a, b)
+    if b == 0 then
+        return a
+    end
+
+    return gcd(b, a % b)
+end
+
+
+local function resolve_srv(client, answers)
+    if #answers == 0 then
+        return nil, "empty SRV record"
+    end
+
+    local resolved_answers = {}
+    local answer_to_count = {}
+    for _, answer in ipairs(answers) do
+        if answer.type ~= client.TYPE_SRV then
+            return nil, "mess SRV with other record"
+        end
+
+        local resolved, err = client.resolve(answer.target)
+        if not resolved then
+            local msg = "failed to resolve SRV record " .. answer.target .. ": " .. err
+            return nil, msg
+        end
+
+        log.info("dns resolve SRV ", answer.target, ", result: ",
+                 json.delay_encode(resolved))
+
+        local weight = answer.weight

Review comment:
       I think the explantation to treat zero weight record to `1` https://github.com/apache/apisix/blob/3af2175629f727d0374b7f77dbdc3a510cf44d81/docs/en/latest/dns.md#srv-record, can also add here.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org