feat: replace events library with shdict by Revolyssup · Pull Request #12353 · apache/apisix

Revolyssup · 2025-06-19T14:09:22Z

Breakout from: #12263
Fixes #
Currently nacos discovery uses event library and during testing it was found out that when using lua-resty-events, sometimes not all workers get the events. And inconsistencies emerge. Moreover the idiomatic way to share data between workers is through a shared dict. Therefore this PR migrates nacos from older events mechanism to newer shared dict way. Now only priviliged agent will fetch data from nacos and write to shdict while all workers will read from it.

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 first)

Revolyssup · 2025-06-19T16:41:27Z

apisix/discovery/nacos/init.lua



 function _M.dump_data()
-    return {config = local_conf.discovery.nacos, services = applications or {}}


config is removed from here as it's a security risk.

make sense.

Revolyssup · 2025-06-19T17:59:12Z

t/discovery/nacos2.t

+            local http = require("resty.http")
+            local uri = "http://127.0.0.1:" .. ngx.var.server_port
+
+            -- Wait for 2 seconds for APISIX initialization


Test is modified to add this sleep because: Since retry mechanism is removed, priviliged agent fills data in shdict might take some time so we want to send the request after that.

apisix/discovery/nacos/init.lua

nic-6443 · 2025-06-20T09:35:35Z

apisix/discovery/nacos/init.lua

+        local value = nacos_dict:get(key)
+        if value then
+            local nodes = core.json.decode(value)
+            if nodes and #nodes > 0 then


dump_data is for debug purpose, if value in shared dict for this key is empty , we should return the real empty value for debugging instead hide it.

nic-6443

one thing you missed: remove unused key from shared dict in every fetch_full_registry run

nic-6443 · 2025-06-20T10:22:47Z

apisix/discovery/nacos/init.lua

+    local old_curr_service_in_use = curr_service_in_use
+    curr_service_in_use = service_names
+    -- remove services that are not in use anymore
+    for key, _ in pairs(old_curr_service_in_use) do
+        if not service_names[key] then
+            nacos_dict:delete(key)
+        end
    end


Suggested change

local old_curr_service_in_use = curr_service_in_use

curr_service_in_use = service_names

-- remove services that are not in use anymore

for key, _ in pairs(old_curr_service_in_use) do

if not service_names[key] then

nacos_dict:delete(key)

end

end

-- remove services that are not in use anymore

for key, _ in pairs(curr_service_in_use) do

if not service_names[key] then

nacos_dict:delete(key)

end

end

curr_service_in_use = service_names

apisix/discovery/nacos/init.lua

Revolyssup · 2025-06-24T08:39:57Z

@bzp2010 I ran a benchmark to test whether the shdict:get brings too much performance degradation. But based on below results I don't think this should be an issue.

Configuration used

config.yaml

apisix:
  node_listen: 1984
deployment:
  role: data_plane
  role_data_plane:
    config_provider: yaml
nginx_config:
  worker_processes: 8
discovery:
  nacos:
      host:
        - "http://nacos:nacos@127.0.0.1:8848"
      prefix: "/nacos/v1/"
      fetch_interval: 1
      weight: 1
      timeout:
        connect: 2000
        send: 2000
        read: 5000

apisix.yaml

routes:
  -
    uri: /hello
    upstream:
      service_name: APISIX-NACOS
      discovery_type: nacos
      type: roundrobin
#END

Here are the results.

On master

❯ wrk -c 10 -t 5 -d 30s -R 50000 http://localhost:1984/hello
Running 30s test @ http://localhost:1984/hello
  5 threads and 10 connections
  Thread calibration: mean lat.: 2999.418ms, rate sampling interval: 10395ms
  Thread calibration: mean lat.: 2885.546ms, rate sampling interval: 10125ms
  Thread calibration: mean lat.: 2857.330ms, rate sampling interval: 10059ms
  Thread calibration: mean lat.: 2967.000ms, rate sampling interval: 10231ms
  Thread calibration: mean lat.: 2953.790ms, rate sampling interval: 10256ms
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    11.12s     3.06s   16.47s    57.49%
    Req/Sec     4.52k    82.05     4.63k    60.00%
  689405 requests in 30.00s, 110.45MB read
Requests/sec:  22980.47
Transfer/sec:      3.68MB

After changes

❯ wrk -c 10 -t 5 -d 30s -R 50000 http://localhost:1984/hello
Running 30s test @ http://localhost:1984/hello
  5 threads and 10 connections
  Thread calibration: mean lat.: 3060.053ms, rate sampling interval: 10584ms
  Thread calibration: mean lat.: 3084.401ms, rate sampling interval: 10772ms
  Thread calibration: mean lat.: 3254.041ms, rate sampling interval: 11198ms
  Thread calibration: mean lat.: 3093.539ms, rate sampling interval: 10985ms
  Thread calibration: mean lat.: 3062.424ms, rate sampling interval: 10772ms
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    11.89s     3.36s   17.74s    56.68%
    Req/Sec     4.13k    39.11     4.18k    60.00%
  623009 requests in 30.00s, 99.81MB read
Requests/sec:  20767.18
Transfer/sec:      3.33MB

Result

The QPS dropped from 22980.47 to 20767.18 with ~2k difference. I think this should be acceptable given the issue with events library this PR solves. According to me, it's an okay tradeoff

membphis

LGTM

lchpersonal · 2025-08-14T09:41:18Z

Breakout from: #12263 Fixes # Currently nacos discovery uses event library and during testing it was found out that when using lua-resty-events, sometimes not all workers get the events. And inconsistencies emerge. Moreover the idiomatic way to share data between workers is through a shared dict. Therefore this PR migrates nacos from older events mechanism to newer shared dict way. Now only priviliged agent will fetch data from nacos and write to shdict while all workers will read from it.

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 first)

How to understand: Now only priviliged agent will fetch data from nacos and write to shdict while all workers will read from it ？

In the current code, does each worker periodically fetch registration information from Nacos?

9268 · 2026-01-06T02:55:37Z

Currently nacos discovery uses event library and during testing it was found out that when using lua-resty-events, sometimes not all workers get the events. And inconsistencies emerge. Moreover the idiomatic way to share data between workers is through a shared dict. Therefore this PR migrates nacos from older events mechanism to newer shared dict way. Now only priviliged agent will fetch data from nacos and write to shdict while all workers will read from it.

@Revolyssup I'm upgrade apisix from 3.7 to 3.14.1，my nacos server get 40x requests than original apisxi（worker=40），it seems every worker is fetching data from nacos，i checked apisix/discovery/nacos/init.lua and not found restriction applied on timer running on special worker， the concurrent of nacos reqs is unacceptable，is this a feature or a bug？

Baoyuantop · 2026-01-07T01:50:19Z

Hi @9268, thank you for your report. After reviewing the code, I found that the Nacos service discovery is currently handling data requests from every worker, which needs to be changed so that only the privileged agent can retrieve the data. Please feel free to submit an issue to document this problem or submit PR modifications.

9268 · 2026-01-12T02:16:30Z

Hi @9268, thank you for your report. After reviewing the code, I found that the Nacos service discovery is currently handling data requests from every worker, which needs to be changed so that only the privileged agent can retrieve the data. Please feel free to submit an issue to document this problem or submit PR modifications.

@Baoyuantop hi，i have commit a pr to fix this issue，pls have a review #12867

Revolyssup added 2 commits June 19, 2025 12:46

feat: replace events library with shdict

7d88305

fix tests

43d4dbc

dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Jun 19, 2025

Revolyssup added 4 commits June 19, 2025 19:40

revert schema

d471a9d

fix tests

866002d

fix lint

a89ae6b

reindex

8da5faa

Revolyssup commented Jun 19, 2025

View reviewed changes

membphis reviewed Jun 20, 2025

View reviewed changes

apisix/discovery/nacos/init.lua Show resolved Hide resolved

check err

011705d

nic-6443 reviewed Jun 20, 2025

View reviewed changes

apply suggestions

b01d66d

nic-6443 reviewed Jun 20, 2025

View reviewed changes

Revolyssup added 3 commits June 20, 2025 15:53

apply suggestions

b23baa1

apply suggestion

0620170

fix lint

ec2271a

nic-6443 approved these changes Jun 20, 2025

View reviewed changes

Revolyssup requested a review from membphis June 20, 2025 14:29

bzp2010 reviewed Jun 21, 2025

View reviewed changes

apisix/discovery/nacos/init.lua Show resolved Hide resolved

bzp2010 approved these changes Jun 24, 2025

View reviewed changes

membphis approved these changes Jun 26, 2025

View reviewed changes

Revolyssup merged commit e2e6829 into apache:master Jun 26, 2025
27 checks passed

Revolyssup deleted the revolyssup/nacos-shdict branch June 26, 2025 08:27

Baoyuantop mentioned this pull request Jun 27, 2025

bug: nacos node failure should not corrupt in-memory data #11252

Closed

9268 mentioned this pull request Jan 7, 2026

feat: optimize nacos discovery with privileged agent and LRU cache #12867

Open

5 tasks



		function _M.dump_data()
		return {config = local_conf.discovery.nacos, services = applications or {}}

Conversation

Revolyssup commented Jun 19, 2025

Checklist

Uh oh!

Revolyssup Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

nic-6443 Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

Revolyssup Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nic-6443 Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

nic-6443 left a comment

Choose a reason for hiding this comment

Uh oh!

nic-6443 Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Revolyssup commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Configuration used

On master

After changes

Result

Uh oh!

membphis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lchpersonal commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

9268 commented Jan 6, 2026

Uh oh!

Baoyuantop commented Jan 7, 2026

Uh oh!

9268 commented Jan 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Revolyssup commented Jun 24, 2025 •

edited

Loading

lchpersonal commented Aug 14, 2025 •

edited

Loading