feat(config_etcd): use a single long http connection to watch all resources #9456

kingluo · 2023-05-10T17:22:19Z

Description

only one http connection to watch the prefix for all resources
use chunked streaming to receive events in the connection, timeout=50 second

Sequence Diagram:

/routes watcher->/routes watcher: first sync_data()
/routes watcher->main watcher: start
/routes watcher->/routes watcher: wait main watcher
/upstreams watcher->/upstreams watcher: first sync_data(), wait main watcher
main watcher->etcd: get current etcd revision and start watch there
main watcher-->/routes watcher: started
main watcher-->/upstreams watcher: started
/routes watcher->etcd: readdir() if need_reloaded
/routes watcher->main watcher: iterate history watch events, found
/upstreams watcher->main watcher: no history event, wait...
etcd-->main watcher: watch event
main watcher->main watcher: save event
main watcher->/upstreams watcher: notify
/upstreams watcher->main watcher: iterate history watch events, found
etcd-->main watcher: timeout
main watcher->etcd: reconnect and restart watch from max received revision
etcd-->main watcher: compacted
main watcher->etcd: reconnect and restart watch from compacted revision

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)

kingluo · 2023-05-11T16:31:48Z

t/plugin/error-log-logger-skywalking.t

@@ -118,8 +118,8 @@ qr/Batch Processor\[error-log-logger\] failed to process entries: error while se
 --- request
 GET /tg
 --- response_body
--- error_log eval
-qr/.*\[\{\"body\":\{\"text\":\{\"text\":\".*this is an error message for test.*\"\}\},\"endpoint\":\"\",\"service\":\"APISIX\",\"serviceInstance\":\"instance\".*/


The new watch subsystem may print new logs.
All in all, strict log assumption is not a good practice.

kingluo · 2023-05-12T03:48:11Z

t/core/etcd-sync.t

--- response_body
-prev_index updated
--- error_log eval
-qr/(create watch stream for key|cancel watch connection success)/


deprecated test. etcd watch is a single long connection now.

kingluo · 2023-05-13T15:50:28Z

ci/linux_openresty_runner.sh

@@ -18,5 +18,5 @@


 export OPENRESTY_VERSION=source
-export TEST_CI_USE_GRPC=true
+#export TEST_CI_USE_GRPC=true


We're going to start focusing on etcd's http tests.

kingluo · 2023-05-13T16:15:14Z

apisix/core/config_etcd.lua

+    watch_ctx.wait_init = nil
+
+    local opts = {}
+    opts.timeout = 50 -- second


The choice of 50 seconds is to make it smaller than the default proxy_read_timeout value, 60 seconds, so that nginx will not print error logs, such as:

2023/05/10 23:43:44 [error] 3668019#3668019: *809 upstream timed out (110: Connection timed out) while reading upstream, client: unix:, server: , request: "POST /v3/watch HTTP/1.1", upstream: "http://127.0.0.1:2379/v3/watch", host: "127.0.0.1"

monkeyDluffy6017 · 2023-05-19T13:38:41Z

apisix/core/config_etcd.lua

+                if err ~= "closed" and
+                    err ~= "timeout" and
+                    err ~= "broken pipe" then
+                    log.error("wait watch event: ", err)
+                end


Suggested change

if err ~= "closed" and

err ~= "timeout" and

err ~= "broken pipe" then

log.error("wait watch event: ", err)

end

if err ~= "closed" and

err ~= "timeout" and

err ~= "broken pipe"

then

log.error("wait watch event: ", err)

end

monkeyDluffy6017 · 2023-05-19T14:06:02Z

apisix/core/config_etcd.lua

+                end
+            end
+
+            for i = 1,min_idx-1 do


Suggested change

for i = 1,min_idx-1 do

for i = 1, min_idx-1 do

monkeyDluffy6017 · 2023-05-19T14:07:09Z

apisix/core/config_etcd.lua

+
+            if min_idx > 100 then
+                for k, idx in pairs(watch_ctx.idx) do
+                    watch_ctx.idx[k] = idx-min_idx+1


Suggested change

watch_ctx.idx[k] = idx-min_idx+1

watch_ctx.idx[k] = idx - min_idx + 1

monkeyDluffy6017 · 2023-05-19T14:09:04Z

apisix/core/config_etcd.lua

+                    watch_ctx.idx[k] = idx-min_idx+1
+                end
+                -- trim the res table
+                for i = 1,min_idx-1 do


Suggested change

for i = 1,min_idx-1 do

for i = 1, min_idx - 1 do

monkeyDluffy6017 · 2023-05-19T14:12:03Z

apisix/core/config_etcd.lua

-        res, err = res_func()
-    end
+    ::iterate_events::
+    for i = watch_ctx.idx[key],#watch_ctx.res do


Suggested change

for i = watch_ctx.idx[key],#watch_ctx.res do

for i = watch_ctx.idx[key], #watch_ctx.res do

monkeyDluffy6017 · 2023-05-19T14:12:22Z

apisix/core/config_etcd.lua

-    end
+    ::iterate_events::
+    for i = watch_ctx.idx[key],#watch_ctx.res do
+        watch_ctx.idx[key] = i+1


Suggested change

watch_ctx.idx[key] = i+1

watch_ctx.idx[key] = i + 1

monkeyDluffy6017 · 2023-05-19T15:42:57Z

Why do you call this "long http connection", the http_cli will be recreated every time: https://github.com/api7/lua-resty-etcd/blob/master/lib/resty/etcd/v3.lua#L803?

kingluo · 2023-05-19T15:50:10Z

Why do you call this "long http connection", the http_cli will be recreated every time: https://github.com/api7/lua-resty-etcd/blob/master/lib/resty/etcd/v3.lua#L803?

        ::watch_event::
        while true do
            local res, err = res_func()
            log.info("res_func: ", inspect(res))

Please read the code carefully.
Once the http connection for watching is created, it keeps receiving the watch event chunks, unless it is timeout or compacted.
So is it long or not?

…d streaming

monkeyDluffy6017 · 2023-05-22T09:14:20Z

apisix/core/config_etcd.lua

+
+        ::watch_event::
+        while true do
+            local res, err = res_func()


It's very good design here, the server response is a stream, if we don't close the connection here, we could get the response event one by one

apisix/core/config_etcd.lua

monkeyDluffy6017 · 2023-05-23T04:15:00Z

apisix/core/config_etcd.lua

+    if log_level >= NGX_INFO then
+        log.info("append res: ", inspect(res), ", err: ", inspect(err))
+    end


Why not use json.delay_encode here?

the delay stuff has bug:

apisix/apisix/core/json.lua

Lines 116 to 120 in a943c03

function _M.delay_encode(data, force)

delay_tab.data = data

delay_tab.force = force

return delay_tab

end

It only uses a singleton table to log, but here I need two vars to log.
And inspect is more informational than json for debugging.

…ources (apache#9456)

kingluo changed the title ~~feat(config_etcd): use only one http connection to watch all resources~~ feat: config_etcd: use only one http connection to watch all resources May 10, 2023

kingluo changed the title ~~feat: config_etcd: use only one http connection to watch all resources~~ feat(config_etcd): use only one http connection to watch all resources May 11, 2023

kingluo changed the title ~~feat(config_etcd): use only one http connection to watch all resources~~ feat(config_etcd): use only one long http connection to watch all resources May 11, 2023

kingluo force-pushed the etcd-http-watch-optimize branch from 66bd5bc to 1c0102f Compare May 11, 2023 06:25

kingluo changed the title ~~feat(config_etcd): use only one long http connection to watch all resources~~ feat(config_etcd): use a single long http connection to watch all resources May 11, 2023

kingluo commented May 11, 2023

View reviewed changes

kingluo commented May 12, 2023

View reviewed changes

kingluo commented May 13, 2023

View reviewed changes

kingluo marked this pull request as ready for review May 13, 2023 16:08

kingluo commented May 13, 2023

View reviewed changes

kingluo requested review from monkeyDluffy6017, leslie-tsang and soulbird May 13, 2023 16:20

monkeyDluffy6017 reviewed May 19, 2023

View reviewed changes

kingluo added 9 commits May 22, 2023 14:33

feat(config_etcd): use only one http connection to watch, with chunke…

f256cfb

…d streaming

fix PR

4c4d64a

fix PR

23a7287

fix PR

55bb0ae

fix PR

2dcf9bc

fix PR

6fc4613

fix PR

d09a6bb

fix PR

36e2a76

code formating fix

760f9e6

kingluo added 2 commits May 22, 2023 14:33

log inspect for ngx.INFO

a9de0f2

fix PR

4eb055b

kingluo force-pushed the etcd-http-watch-optimize branch from 683f936 to 4eb055b Compare May 22, 2023 06:33

monkeyDluffy6017 reviewed May 22, 2023

View reviewed changes

monkeyDluffy6017 reviewed May 23, 2023

View reviewed changes

apisix/core/config_etcd.lua Outdated Show resolved Hide resolved

monkeyDluffy6017 reviewed May 23, 2023

View reviewed changes

apisix/core/config_etcd.lua Show resolved Hide resolved

fix PR

70f643b

monkeyDluffy6017 reviewed May 23, 2023

View reviewed changes

leslie-tsang approved these changes May 23, 2023

View reviewed changes

monkeyDluffy6017 approved these changes May 23, 2023

View reviewed changes

monkeyDluffy6017 merged commit 4377d05 into apache:master May 23, 2023

leslie-tsang added the need backport label Jun 28, 2023

hansedong mentioned this pull request Jul 4, 2023

bug: error_log report error in the way apisix connected to etcd using gRPC #9336

Closed

AlinsRan pushed a commit to AlinsRan/apisix that referenced this pull request Jul 7, 2023

feat(config_etcd): use a single long http connection to watch all res…

30a7f4c

…ources (apache#9456)

This was referenced Jul 11, 2023

fix: update_count is reset once updated, cause cache key conflict #9811

Merged

bug: config_util.lua:99: attempt to index local 'item' (a boolean value) #9822

Closed

wklken mentioned this pull request Aug 2, 2023

bug: route 404 after upgrade to 3.2.2 #9951

Closed

jiangfucheng mentioned this pull request Aug 8, 2023

fix: worker not exited when executing quit or reload command #9909

Merged

5 tasks

monkeyDluffy6017 mentioned this pull request Dec 7, 2023

fix: memory leak caused by timer that never quit #10614

Merged

5 tasks

wklken mentioned this pull request Dec 8, 2023

help request: memory growing up #10392

Closed

sheharyaar mentioned this pull request Feb 18, 2024

fix: remove grpc-client-nginx-module api7/apisix-build-tools#370

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(config_etcd): use a single long http connection to watch all resources #9456

feat(config_etcd): use a single long http connection to watch all resources #9456

kingluo commented May 10, 2023 •

edited

Loading

kingluo May 11, 2023

kingluo May 12, 2023

kingluo May 13, 2023 •

edited

Loading

kingluo May 13, 2023 •

edited

Loading

monkeyDluffy6017 May 19, 2023

monkeyDluffy6017 May 19, 2023

monkeyDluffy6017 May 19, 2023

monkeyDluffy6017 May 19, 2023

monkeyDluffy6017 May 19, 2023

monkeyDluffy6017 May 19, 2023

monkeyDluffy6017 commented May 19, 2023

kingluo commented May 19, 2023 •

edited

Loading

monkeyDluffy6017 May 22, 2023

monkeyDluffy6017 May 23, 2023

kingluo May 23, 2023

	watch_ctx.idx[k] = idx-min_idx+1
	watch_ctx.idx[k] = idx - min_idx + 1

	for i = watch_ctx.idx[key],#watch_ctx.res do
	for i = watch_ctx.idx[key], #watch_ctx.res do

	function _M.delay_encode(data, force)
	delay_tab.data = data
	delay_tab.force = force
	return delay_tab
	end

feat(config_etcd): use a single long http connection to watch all resources #9456

feat(config_etcd): use a single long http connection to watch all resources #9456

Conversation

kingluo commented May 10, 2023 • edited Loading

Description

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kingluo May 13, 2023 • edited Loading

Choose a reason for hiding this comment

kingluo May 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monkeyDluffy6017 commented May 19, 2023

kingluo commented May 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kingluo commented May 10, 2023 •

edited

Loading

kingluo May 13, 2023 •

edited

Loading

kingluo May 13, 2023 •

edited

Loading

kingluo commented May 19, 2023 •

edited

Loading