You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "SkyeYoung (via GitHub)" <gi...@apache.org> on 2026/01/14 06:17:31 UTC

[PR] fix: request failure during reload after any Eureka node fails [apisix]

SkyeYoung opened a new pull request, #12906:
URL: https://github.com/apache/apisix/pull/12906

   ### Description
   
   <!-- Please include a summary of the change and which issue is fixed. -->
   <!-- Please also include relevant motivation and context. -->
   
   #### Which issue(s) this PR fixes:
   <!--
   *Automatically closes linked issue when PR is merged.
   Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
   -->
   Fixes #
   
   ### Checklist
   
   - [ ] I have explained the need for this PR and the problem it solves
   - [ ] I have explained the changes or the new features added to this PR
   - [ ] I have added tests corresponding to this change
   - [ ] I have updated the documentation to reflect this change
   - [ ] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
   
   <!--
   
   Note
   
   1. Mark the PR as draft until it's ready to be reviewed.
   2. Always add/update tests for any changes unless you have a good reason.
   3. Always update the documentation to reflect the changes made in the PR.
   4. Make a new commit to resolve conversations instead of `push -f`.
   5. To resolve merge conflicts, merge master instead of rebasing.
   6. Use "request review" to notify the reviewer after making changes.
   7. Only a reviewer can mark a conversation as resolved.
   
   -->
   


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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


Re: [PR] fix: request failure during reload after any Eureka node fails [apisix]

Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on code in PR #12906:
URL: https://github.com/apache/apisix/pull/12906#discussion_r2693645179


##########
apisix/discovery/eureka/init.lua:
##########
@@ -145,26 +148,36 @@ local function fetch_full_registry(premature)
         return
     end
 
-    local request_uri, basic_auth = service_info()
-    if not request_uri then
+    local endpoints = build_endpoints()
+    if not endpoints or #endpoints == 0 then
         return
     end
 
-    local res, err = request(request_uri, basic_auth, "GET", "apps")
-    if not res then
-        log.error("failed to fetch registry", err)
-        return
+    -- try endpoints from random position, failover on error
+    local selected_endpoint
+    local selected_body
+    local num_endpoints = #endpoints
+    local start = math_random(num_endpoints)
+    for i = 0, num_endpoints - 1 do
+        local endpoint = endpoints[(start + i - 1) % num_endpoints + 1]

Review Comment:
   num_endpoints = 3,start = 1
   
   i | (start + i - 1) % 3 + 1 | (start + i) % 3
   -- | -- | --
   0 | (1 + 0 - 1) % 3 + 1 = 1 | (1 + 0) % 3 = 1
   1 | (1 + 1 - 1) % 3 + 1 = 2 | (1 + 1) % 3 = 2
   2 | (1 + 2 - 1) % 3 + 1 = 3 | (1 + 2) % 3 = 0
   



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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


Re: [PR] fix: request failure during reload after any Eureka node fails [apisix]

Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on code in PR #12906:
URL: https://github.com/apache/apisix/pull/12906#discussion_r2693724311


##########
apisix/discovery/eureka/init.lua:
##########
@@ -39,34 +39,37 @@ local _M = {
 }
 
 
-local function service_info()
-    local host = local_conf.discovery and
+-- build all eureka endpoints from config
+local function build_endpoints()
+    local host_list = local_conf.discovery and
         local_conf.discovery.eureka and local_conf.discovery.eureka.host
-    if not host then
+    if not host_list or #host_list == 0 then
         log.error("do not set eureka.host")
-        return
-    end
-
-    local basic_auth
-    -- TODO Add health check to get healthy nodes.
-    local url = host[math_random(#host)]
-    local auth_idx = str_find(url, "@")
-    if auth_idx then
-        local protocol_idx = str_find(url, "://")
-        local protocol = string_sub(url, 1, protocol_idx + 2)
-        local user_and_password = string_sub(url, protocol_idx + 3, auth_idx - 1)
-        local other = string_sub(url, auth_idx + 1)
-        url = protocol .. other
-        basic_auth = "Basic " .. ngx.encode_base64(user_and_password)
-    end
-    if local_conf.discovery.eureka.prefix then
-        url = url .. local_conf.discovery.eureka.prefix
-    end
-    if string_sub(url, #url) ~= "/" then
-        url = url .. "/"
+        return nil
+    end
+
+    local built_endpoints = core.table.new(#host_list, 0)

Review Comment:
   The current calling relationship is as follows:
   
   fetch_full_registry -> build_endpoints
   
   ```lua
   function _M.init_worker()
       ...
       ngx_timer_at(0, fetch_full_registry)
       ngx_timer_every(fetch_interval, fetch_full_registry)
   end
   ```



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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


Re: [PR] fix: request failure during reload after any Eureka node fails [apisix]

Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on code in PR #12906:
URL: https://github.com/apache/apisix/pull/12906#discussion_r2693733517


##########
apisix/discovery/eureka/init.lua:
##########
@@ -145,26 +148,36 @@ local function fetch_full_registry(premature)
         return
     end
 
-    local request_uri, basic_auth = service_info()
-    if not request_uri then
+    local endpoints = build_endpoints()
+    if not endpoints or #endpoints == 0 then
         return
     end
 
-    local res, err = request(request_uri, basic_auth, "GET", "apps")
-    if not res then
-        log.error("failed to fetch registry", err)
-        return
+    -- try endpoints from random position, failover on error
+    local selected_endpoint
+    local selected_body
+    local num_endpoints = #endpoints
+    local start = math_random(num_endpoints)
+    for i = 0, num_endpoints - 1 do
+        local endpoint = endpoints[(start + i - 1) % num_endpoints + 1]
+        local r, e = request(endpoint.url, endpoint.auth, "GET", "apps")
+        if r and r.body and r.status == 200 then
+            selected_endpoint = endpoint
+            selected_body = r.body
+            break
+        end
+        log.error("failed to fetch registry from ", endpoint.url, ": ",
+                 e or (r and ("status=" .. tostring(r.status)) or "unknown"))
     end
 
-    if not res.body or res.status ~= 200 then
-        log.error("failed to fetch registry, status = ", res.status)
+    if not selected_endpoint then
+        log.error("failed to fetch registry from all eureka hosts")

Review Comment:
   ok. 



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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


Re: [PR] fix: request failure during reload after any Eureka node fails [apisix]

Posted by "membphis (via GitHub)" <gi...@apache.org>.
membphis commented on code in PR #12906:
URL: https://github.com/apache/apisix/pull/12906#discussion_r2693630237


##########
apisix/discovery/eureka/init.lua:
##########
@@ -145,26 +148,36 @@ local function fetch_full_registry(premature)
         return
     end
 
-    local request_uri, basic_auth = service_info()
-    if not request_uri then
+    local endpoints = build_endpoints()
+    if not endpoints or #endpoints == 0 then
         return
     end
 
-    local res, err = request(request_uri, basic_auth, "GET", "apps")
-    if not res then
-        log.error("failed to fetch registry", err)
-        return
+    -- try endpoints from random position, failover on error
+    local selected_endpoint
+    local selected_body
+    local num_endpoints = #endpoints
+    local start = math_random(num_endpoints)
+    for i = 0, num_endpoints - 1 do
+        local endpoint = endpoints[(start + i - 1) % num_endpoints + 1]
+        local r, e = request(endpoint.url, endpoint.auth, "GET", "apps")
+        if r and r.body and r.status == 200 then
+            selected_endpoint = endpoint
+            selected_body = r.body
+            break
+        end
+        log.warn("failed to fetch registry from ", endpoint.url, ": ",

Review Comment:
   should use error log level



##########
apisix/discovery/eureka/init.lua:
##########
@@ -145,26 +148,36 @@ local function fetch_full_registry(premature)
         return
     end
 
-    local request_uri, basic_auth = service_info()
-    if not request_uri then
+    local endpoints = build_endpoints()
+    if not endpoints or #endpoints == 0 then
         return
     end
 
-    local res, err = request(request_uri, basic_auth, "GET", "apps")
-    if not res then
-        log.error("failed to fetch registry", err)
-        return
+    -- try endpoints from random position, failover on error
+    local selected_endpoint
+    local selected_body
+    local num_endpoints = #endpoints
+    local start = math_random(num_endpoints)
+    for i = 0, num_endpoints - 1 do
+        local endpoint = endpoints[(start + i - 1) % num_endpoints + 1]

Review Comment:
   ```suggestion
           local endpoint = endpoints[(start + i) % num_endpoints]
   ```



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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


Re: [PR] fix: request failure during reload after any Eureka node fails [apisix]

Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on code in PR #12906:
URL: https://github.com/apache/apisix/pull/12906#discussion_r2693654369


##########
apisix/discovery/eureka/init.lua:
##########
@@ -145,26 +148,36 @@ local function fetch_full_registry(premature)
         return
     end
 
-    local request_uri, basic_auth = service_info()
-    if not request_uri then
+    local endpoints = build_endpoints()
+    if not endpoints or #endpoints == 0 then
         return
     end
 
-    local res, err = request(request_uri, basic_auth, "GET", "apps")
-    if not res then
-        log.error("failed to fetch registry", err)
-        return
+    -- try endpoints from random position, failover on error
+    local selected_endpoint
+    local selected_body
+    local num_endpoints = #endpoints
+    local start = math_random(num_endpoints)
+    for i = 0, num_endpoints - 1 do
+        local endpoint = endpoints[(start + i - 1) % num_endpoints + 1]

Review Comment:
   Is the list not `1-based`?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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


Re: [PR] fix: request failure during reload after any Eureka node fails [apisix]

Posted by "bzp2010 (via GitHub)" <gi...@apache.org>.
bzp2010 commented on code in PR #12906:
URL: https://github.com/apache/apisix/pull/12906#discussion_r2693237412


##########
apisix/discovery/eureka/init.lua:
##########
@@ -140,31 +109,75 @@ local function parse_instance(instance)
 end
 
 
+-- build all eureka endpoints from config
+local function build_endpoints()
+    local host_list = local_conf.discovery and
+        local_conf.discovery.eureka and local_conf.discovery.eureka.host
+    if not host_list or #host_list == 0 then
+        log.error("do not set eureka.host")
+        return nil
+    end
+
+    local built_endpoints = core.table.new(#host_list, 0)
+    for _, h in ipairs(host_list) do
+        local url = h
+        local basic_auth
+        local auth_idx = str_find(url, "@")
+        if auth_idx then
+            local protocol_idx = str_find(url, "://")
+            local protocol = string_sub(url, 1, protocol_idx + 2)
+            local user_and_password = string_sub(url, protocol_idx + 3, auth_idx - 1)
+            local other = string_sub(url, auth_idx + 1)
+            url = protocol .. other
+            basic_auth = "Basic " .. ngx.encode_base64(user_and_password)
+        end
+        if local_conf.discovery.eureka.prefix then
+            url = url .. local_conf.discovery.eureka.prefix
+        end
+        if string_sub(url, #url) ~= "/" then
+            url = url .. "/"
+        end
+        core.table.insert(built_endpoints, { url = url, auth = basic_auth })
+    end
+    return built_endpoints
+end
+
+
 local function fetch_full_registry(premature)
     if premature then
         return
     end
 
-    local request_uri, basic_auth = service_info()
-    if not request_uri then
+    local endpoints = build_endpoints()
+    if not endpoints or #endpoints == 0 then
         return
     end
 
-    local res, err = request(request_uri, basic_auth, "GET", "apps")
-    if not res then
-        log.error("failed to fetch registry", err)
-        return
+    -- try endpoints from random position, failover on error
+    local selected_endpoint
+    local selected_body
+    local num_endpoints = #endpoints
+    local start = math_random(num_endpoints)
+    for i = 0, num_endpoints - 1 do
+        local endpoint = endpoints[(start + i - 1) % num_endpoints + 1]
+        local r, e = request(endpoint.url, endpoint.auth, "GET", "apps")
+        if r and r.body and r.status == 200 then
+            selected_endpoint = endpoint
+            selected_body = r.body
+            break
+        end

Review Comment:
   This modification essentially ensures that if a registry unexpectedly fails, it will retry until a good registry is used. While this may introduce some additional latency, is it necessary to document it?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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


Re: [PR] fix: request failure during reload after any Eureka node fails [apisix]

Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on code in PR #12906:
URL: https://github.com/apache/apisix/pull/12906#discussion_r2693291034


##########
apisix/discovery/eureka/init.lua:
##########
@@ -140,31 +109,75 @@ local function parse_instance(instance)
 end
 
 
+-- build all eureka endpoints from config
+local function build_endpoints()
+    local host_list = local_conf.discovery and
+        local_conf.discovery.eureka and local_conf.discovery.eureka.host
+    if not host_list or #host_list == 0 then
+        log.error("do not set eureka.host")
+        return nil
+    end
+
+    local built_endpoints = core.table.new(#host_list, 0)
+    for _, h in ipairs(host_list) do
+        local url = h
+        local basic_auth
+        local auth_idx = str_find(url, "@")
+        if auth_idx then
+            local protocol_idx = str_find(url, "://")
+            local protocol = string_sub(url, 1, protocol_idx + 2)
+            local user_and_password = string_sub(url, protocol_idx + 3, auth_idx - 1)
+            local other = string_sub(url, auth_idx + 1)
+            url = protocol .. other
+            basic_auth = "Basic " .. ngx.encode_base64(user_and_password)
+        end
+        if local_conf.discovery.eureka.prefix then
+            url = url .. local_conf.discovery.eureka.prefix
+        end
+        if string_sub(url, #url) ~= "/" then
+            url = url .. "/"
+        end
+        core.table.insert(built_endpoints, { url = url, auth = basic_auth })
+    end
+    return built_endpoints
+end
+
+
 local function fetch_full_registry(premature)
     if premature then
         return
     end
 
-    local request_uri, basic_auth = service_info()
-    if not request_uri then
+    local endpoints = build_endpoints()
+    if not endpoints or #endpoints == 0 then
         return
     end
 
-    local res, err = request(request_uri, basic_auth, "GET", "apps")
-    if not res then
-        log.error("failed to fetch registry", err)
-        return
+    -- try endpoints from random position, failover on error
+    local selected_endpoint
+    local selected_body
+    local num_endpoints = #endpoints
+    local start = math_random(num_endpoints)
+    for i = 0, num_endpoints - 1 do
+        local endpoint = endpoints[(start + i - 1) % num_endpoints + 1]
+        local r, e = request(endpoint.url, endpoint.auth, "GET", "apps")
+        if r and r.body and r.status == 200 then
+            selected_endpoint = endpoint
+            selected_body = r.body
+            break
+        end

Review Comment:
   Perhaps we should add a similar description to all service discovery documentation.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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


Re: [PR] fix: request failure during reload after any Eureka node fails [apisix]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12906:
URL: https://github.com/apache/apisix/pull/12906#discussion_r2692749042


##########
apisix/discovery/eureka/init.lua:
##########
@@ -192,6 +202,9 @@ local function fetch_full_registry(premature)
         end
     end
     applications = up_apps
+    log.info("successfully updated service registry, services count=",
+             core.table.nkeys(up_apps), "; source=",
+             selected_endpoint and selected_endpoint.url or "unknown")

Review Comment:
   The condition `selected_endpoint and selected_endpoint.url or 'unknown'` is redundant since this log line is only reached when `selected_endpoint` is not nil (line 170 returns early if nil). Simplify to just `selected_endpoint.url`.
   ```suggestion
                selected_endpoint.url)
   ```



##########
apisix/discovery/eureka/init.lua:
##########
@@ -210,6 +223,9 @@ function _M.init_worker()
     log.info("default_weight:", default_weight, ".")
     local fetch_interval = local_conf.discovery.eureka.fetch_interval or 30
     log.info("fetch_interval:", fetch_interval, ".")
+
+    endpoints = build_endpoints()

Review Comment:
   If `build_endpoints()` returns `nil` due to missing configuration, the timers will still be started, causing them to fail silently on every execution. Consider adding error handling to prevent timer initialization when endpoints cannot be built, similar to how other discovery services handle init failures.
   ```suggestion
       endpoints = build_endpoints()
       if not endpoints or #endpoints == 0 then
           log.error("failed to init eureka discovery: no valid endpoints " ..
                     "could be built, please check discovery.eureka configuration")
           return
       end
   ```



##########
t/discovery/eureka.t:
##########
@@ -115,3 +115,53 @@ default_weight:80.
 fetch_interval:10.
 eureka uri:http://127.0.0.1:8761/eureka/.
 connect_timeout:1500, send_timeout:1500, read_timeout:1500.
+
+
+
+=== TEST 4: fallback to next eureka host when current host fails
+--- yaml_config
+apisix:
+    node_listen: 1984
+deployment:
+    role: data_plane
+    role_data_plane:
+        config_provider: yaml
+discovery:
+    eureka:
+        host:
+            - "http://127.0.0.1:20997"
+            - "http://127.0.0.1:8761"
+        prefix: "/eureka/"
+        fetch_interval: 1
+        weight: 80
+        timeout:
+            connect: 1500
+            send: 1500
+            read: 1500
+--- apisix_yaml
+routes:
+    -
+        uri: /eureka/*
+        upstream:
+            service_name: APISIX-EUREKA
+            discovery_type: eureka
+            type: roundrobin
+#END
+--- http_config
+    server {
+        listen 20997;
+
+        location / {
+            return 502;
+        }
+    }
+--- request
+GET /eureka/apps/APISIX-EUREKA
+--- response_body_like
+.*<name>APISIX-EUREKA</name>.*

Review Comment:
   The test verifies the response body but doesn't verify that the request was actually routed through the working endpoint (127.0.0.1:8761) rather than the failing one (127.0.0.1:20997). Consider adding an assertion to check that the upstream actually received the request.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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


Re: [PR] fix: request failure during reload after any Eureka node fails [apisix]

Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung merged PR #12906:
URL: https://github.com/apache/apisix/pull/12906


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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


Re: [PR] fix: request failure during reload after any Eureka node fails [apisix]

Posted by "membphis (via GitHub)" <gi...@apache.org>.
membphis commented on code in PR #12906:
URL: https://github.com/apache/apisix/pull/12906#discussion_r2693728184


##########
apisix/discovery/eureka/init.lua:
##########
@@ -145,26 +148,36 @@ local function fetch_full_registry(premature)
         return
     end
 
-    local request_uri, basic_auth = service_info()
-    if not request_uri then
+    local endpoints = build_endpoints()
+    if not endpoints or #endpoints == 0 then
         return
     end
 
-    local res, err = request(request_uri, basic_auth, "GET", "apps")
-    if not res then
-        log.error("failed to fetch registry", err)
-        return
+    -- try endpoints from random position, failover on error
+    local selected_endpoint
+    local selected_body
+    local num_endpoints = #endpoints
+    local start = math_random(num_endpoints)
+    for i = 0, num_endpoints - 1 do
+        local endpoint = endpoints[(start + i - 1) % num_endpoints + 1]
+        local r, e = request(endpoint.url, endpoint.auth, "GET", "apps")
+        if r and r.body and r.status == 200 then
+            selected_endpoint = endpoint
+            selected_body = r.body
+            break
+        end
+        log.error("failed to fetch registry from ", endpoint.url, ": ",
+                 e or (r and ("status=" .. tostring(r.status)) or "unknown"))
     end
 
-    if not res.body or res.status ~= 200 then
-        log.error("failed to fetch registry, status = ", res.status)
+    if not selected_endpoint then
+        log.error("failed to fetch registry from all eureka hosts")

Review Comment:
   need more information, eg: no healthy registry, the count of all hosts 



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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


Re: [PR] fix: request failure during reload after any Eureka node fails [apisix]

Posted by "moonming (via GitHub)" <gi...@apache.org>.
moonming commented on code in PR #12906:
URL: https://github.com/apache/apisix/pull/12906#discussion_r2693630226


##########
apisix/discovery/eureka/init.lua:
##########
@@ -39,34 +39,37 @@ local _M = {
 }
 
 
-local function service_info()
-    local host = local_conf.discovery and
+-- build all eureka endpoints from config
+local function build_endpoints()
+    local host_list = local_conf.discovery and
         local_conf.discovery.eureka and local_conf.discovery.eureka.host
-    if not host then
+    if not host_list or #host_list == 0 then
         log.error("do not set eureka.host")
-        return
-    end
-
-    local basic_auth
-    -- TODO Add health check to get healthy nodes.
-    local url = host[math_random(#host)]
-    local auth_idx = str_find(url, "@")
-    if auth_idx then
-        local protocol_idx = str_find(url, "://")
-        local protocol = string_sub(url, 1, protocol_idx + 2)
-        local user_and_password = string_sub(url, protocol_idx + 3, auth_idx - 1)
-        local other = string_sub(url, auth_idx + 1)
-        url = protocol .. other
-        basic_auth = "Basic " .. ngx.encode_base64(user_and_password)
-    end
-    if local_conf.discovery.eureka.prefix then
-        url = url .. local_conf.discovery.eureka.prefix
-    end
-    if string_sub(url, #url) ~= "/" then
-        url = url .. "/"
+        return nil
+    end
+
+    local built_endpoints = core.table.new(#host_list, 0)

Review Comment:
   we can cache this table instead of create a new one for every request.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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


Re: [PR] fix: request failure during reload after any Eureka node fails [apisix]

Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on code in PR #12906:
URL: https://github.com/apache/apisix/pull/12906#discussion_r2693217833


##########
apisix/discovery/eureka/init.lua:
##########
@@ -210,6 +225,7 @@ function _M.init_worker()
     log.info("default_weight:", default_weight, ".")
     local fetch_interval = local_conf.discovery.eureka.fetch_interval or 30
     log.info("fetch_interval:", fetch_interval, ".")
+

Review Comment:
   ```suggestion
   ```



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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


Re: [PR] fix: request failure during reload after any Eureka node fails [apisix]

Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on code in PR #12906:
URL: https://github.com/apache/apisix/pull/12906#discussion_r2693705047


##########
apisix/discovery/eureka/init.lua:
##########
@@ -145,26 +148,36 @@ local function fetch_full_registry(premature)
         return
     end
 
-    local request_uri, basic_auth = service_info()
-    if not request_uri then
+    local endpoints = build_endpoints()
+    if not endpoints or #endpoints == 0 then
         return
     end
 
-    local res, err = request(request_uri, basic_auth, "GET", "apps")
-    if not res then
-        log.error("failed to fetch registry", err)
-        return
+    -- try endpoints from random position, failover on error
+    local selected_endpoint
+    local selected_body
+    local num_endpoints = #endpoints
+    local start = math_random(num_endpoints)
+    for i = 0, num_endpoints - 1 do
+        local endpoint = endpoints[(start + i - 1) % num_endpoints + 1]
+        local r, e = request(endpoint.url, endpoint.auth, "GET", "apps")
+        if r and r.body and r.status == 200 then
+            selected_endpoint = endpoint
+            selected_body = r.body
+            break
+        end
+        log.warn("failed to fetch registry from ", endpoint.url, ": ",

Review Comment:
   done



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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


Re: [PR] fix: request failure during reload after any Eureka node fails [apisix]

Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on code in PR #12906:
URL: https://github.com/apache/apisix/pull/12906#discussion_r2693654369


##########
apisix/discovery/eureka/init.lua:
##########
@@ -145,26 +148,36 @@ local function fetch_full_registry(premature)
         return
     end
 
-    local request_uri, basic_auth = service_info()
-    if not request_uri then
+    local endpoints = build_endpoints()
+    if not endpoints or #endpoints == 0 then
         return
     end
 
-    local res, err = request(request_uri, basic_auth, "GET", "apps")
-    if not res then
-        log.error("failed to fetch registry", err)
-        return
+    -- try endpoints from random position, failover on error
+    local selected_endpoint
+    local selected_body
+    local num_endpoints = #endpoints
+    local start = math_random(num_endpoints)
+    for i = 0, num_endpoints - 1 do
+        local endpoint = endpoints[(start + i - 1) % num_endpoints + 1]

Review Comment:
   Is the list no `1-based`?



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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


Re: [PR] fix: request failure during reload after any Eureka node fails [apisix]

Posted by "membphis (via GitHub)" <gi...@apache.org>.
membphis commented on code in PR #12906:
URL: https://github.com/apache/apisix/pull/12906#discussion_r2693714162


##########
apisix/discovery/eureka/init.lua:
##########
@@ -145,26 +148,36 @@ local function fetch_full_registry(premature)
         return
     end
 
-    local request_uri, basic_auth = service_info()
-    if not request_uri then
+    local endpoints = build_endpoints()
+    if not endpoints or #endpoints == 0 then
         return
     end
 
-    local res, err = request(request_uri, basic_auth, "GET", "apps")
-    if not res then
-        log.error("failed to fetch registry", err)
-        return
+    -- try endpoints from random position, failover on error
+    local selected_endpoint
+    local selected_body
+    local num_endpoints = #endpoints
+    local start = math_random(num_endpoints)
+    for i = 0, num_endpoints - 1 do
+        local endpoint = endpoints[(start + i - 1) % num_endpoints + 1]

Review Comment:
   your way right, nine 



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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