feat(xds): using data written by xds to control dp behavior #6759

tzssangglass · 2022-03-31T02:51:00Z

Description

This PR is designed to mock the behavior of xds writing data and reading it from ngx.shared.DICT to complete the request proxy.

It needs to accomplish the following functions:

fetch the latest data version number every 1s
update the data in full from ngx.shared.DICT if a new version is available
handle route and upstream data correctly, as if it were subscribed to them from etcd

Fixes # (issue)

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)

tzssangglass · 2022-03-31T15:04:54Z

I want to write some test cases, such as without any keys, json with wrong format, json that does not conform to the route's json scheme, etc., but I don't know how to control the data passed with libxds.so in the test cases.

But I can write data to shdict directly in the test case, which seems more convenient than writing data with libxds.so, is this acceptable way of testing.

cc @spacewander @membphis

t/xds-library/main.go

spacewander · 2022-04-02T01:52:30Z

t/xds-library/config_xds_2.t

+    }
+--- error_code: 404
+--- no_error_log
+[alert]


Why should we check the alert log? Is there any alert log in the path?

because this test case throws an error level log, but the above defines

if (!$block->no_error_log) { $block->set_value("no_error_log", "[error]\n[alert]"); }

so I need to override the above definition with the --- no_error_log statement, and I need to verify that the error level log.

@tzssangglass
We can check like this?

apisix/t/plugin/gzip.t

Line 40 in 97bcb56

if ((!defined $block->error_log) && (!defined $block->no_error_log)) {

We can safely remove it, right?

apisix/cli/ngx_tpl.lua

t/APISIX.pm

t/xds-library/config_xds_2.t

membphis

LGTM

spacewander · 2022-04-04T16:15:00Z

apisix/init.lua

@@ -116,18 +116,18 @@ function _M.http_init_worker()

    require("apisix.debug").init_worker()

+    if core.config == require("apisix.core.config_xds") then


Can we just put all the init_worker together?

For config_xds, I need it to run before route, service is initialised. So do I need to advance config_yaml to the same level as config_xds?

spacewander · 2022-04-04T16:17:12Z

t/xds-library/config_xds_2.t

+    }
+--- error_code: 404
+--- no_error_log
+[alert]


We can safely remove it, right?

spacewander · 2022-04-04T16:23:09Z

apisix/core/config_xds.lua

+
+    if not keys or #keys <= 0 then
+        -- xds did not write any data to shdict
+        return true, "no keys"


Suggested change

return true, "no keys"

return false, "no keys"

spacewander · 2022-04-04T16:23:18Z

apisix/core/config_xds.lua

+    end
+
+    -- v1 version we only support route/upstream
+    local capacity = math_ceil(#keys / 2)


Why divide 2?

ngx.shared["xds-config"] stores all routes, upstreams, here I estimate the capacity of the route by using the total capacity / 2.

Route and upstream are not 1:1.
Maybe we can use #keys directly

spacewander · 2022-04-04T16:27:29Z

apisix/core/config_xds.lua

+
+        -- blocking until xds completes initial configuration
+        while true do
+            fetch_version()


Need a sleep here?

ngx.sleep can not used in init_worker_by_lua, is there another way?

We can use another sleep?

apisix/apisix/core/os.lua

Line 41 in 07d535d

int usleep(useconds_t usec);

spacewander · 2022-04-04T16:28:49Z

apisix/core/config_xds.lua

+
+
+local function sync_data(self)
+    if not latest_version then


As we already block for the version, we can skip this check?

spacewander · 2022-04-06T06:51:58Z

apisix/core/config_xds.lua

+
+local sync_data
+local latest_version
+sync_data = function(self)


Why not use local function xxx?

spacewander · 2022-04-06T06:59:27Z

apisix/core/config_xds.lua

+
+        -- blocking until xds completes initial configuration
+        while true do
+            fetch_version()


We can use another sleep?

apisix/apisix/core/os.lua

Line 41 in 07d535d

int usleep(useconds_t usec);

)

tzssangglass added 4 commits March 30, 2022 09:50

preview

89b001e

revert

d5f488a

revert2

7c7532a

feat(xds): proxy request using data written by xds

5e546ab

tzssangglass marked this pull request as draft March 31, 2022 02:51

tzssangglass added 3 commits March 31, 2022 11:45

fix code lint

94e3f1f

optimize

2065242

add comments

49872ed

tzssangglass added 3 commits April 1, 2022 09:24

Merge branch 'master' into xds2

16c89b9

fix code liint

8aa02d3

add test cases

af9f13a

tzssangglass marked this pull request as ready for review April 1, 2022 10:29

fix CI

0db4cb4

tzssangglass requested review from bzp2010, membphis, shuaijinchao and spacewander April 1, 2022 12:20

spacewander reviewed Apr 2, 2022

View reviewed changes

fix type

2ee27b6

membphis reviewed Apr 2, 2022

View reviewed changes

apisix/cli/ngx_tpl.lua Outdated Show resolved Hide resolved

t/APISIX.pm Outdated Show resolved Hide resolved

fix code revoew

8e62d12

membphis reviewed Apr 3, 2022

View reviewed changes

t/xds-library/config_xds_2.t Show resolved Hide resolved

t/xds-library/config_xds_2.t Show resolved Hide resolved

membphis previously approved these changes Apr 3, 2022

View reviewed changes

spacewander reviewed Apr 4, 2022

View reviewed changes

fix code review

0e3dc22

tzssangglass dismissed membphis’s stale review via 0e3dc22 April 6, 2022 01:42

ficx code review

21e1c5f

spacewander reviewed Apr 6, 2022

View reviewed changes

fix code review

0f58e31

spacewander approved these changes Apr 7, 2022

View reviewed changes

tzssangglass requested a review from membphis April 7, 2022 02:32

membphis approved these changes Apr 7, 2022

View reviewed changes

spacewander merged commit fc3a1c9 into apache:master Apr 7, 2022

Liu-Junlin pushed a commit to Liu-Junlin/apisix that referenced this pull request May 20, 2022

feat(xds): using data written by xds to control dp behavior (apache#6759

1dc8527

)

tzssangglass deleted the xds2 branch May 24, 2022 06:25

spacewander pushed a commit that referenced this pull request Jun 30, 2022

feat(xds): using data written by xds to control dp behavior (#6759)

f3884a9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(xds): using data written by xds to control dp behavior #6759

feat(xds): using data written by xds to control dp behavior #6759

tzssangglass commented Mar 31, 2022 •

edited

Loading

tzssangglass commented Mar 31, 2022

spacewander Apr 2, 2022

tzssangglass Apr 2, 2022 •

edited

Loading

spacewander Apr 2, 2022

tzssangglass Apr 2, 2022

spacewander Apr 4, 2022

membphis left a comment

spacewander Apr 4, 2022

tzssangglass Apr 6, 2022

spacewander Apr 6, 2022

tzssangglass Apr 6, 2022

spacewander Apr 4, 2022

spacewander Apr 4, 2022

tzssangglass Apr 6, 2022

spacewander Apr 4, 2022

tzssangglass Apr 6, 2022

spacewander Apr 6, 2022

tzssangglass Apr 6, 2022

spacewander Apr 4, 2022

tzssangglass Apr 6, 2022

spacewander Apr 6, 2022

tzssangglass Apr 6, 2022

spacewander Apr 4, 2022

tzssangglass Apr 6, 2022

spacewander Apr 6, 2022

tzssangglass Apr 6, 2022

spacewander Apr 6, 2022

		@@ -116,18 +116,18 @@ function _M.http_init_worker()

		require("apisix.debug").init_worker()

		if core.config == require("apisix.core.config_xds") then

feat(xds): using data written by xds to control dp behavior #6759

feat(xds): using data written by xds to control dp behavior #6759

Conversation

tzssangglass commented Mar 31, 2022 • edited Loading

Description

Checklist

tzssangglass commented Mar 31, 2022

Choose a reason for hiding this comment

tzssangglass Apr 2, 2022 • 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

membphis left a 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

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

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

Choose a reason for hiding this comment

tzssangglass commented Mar 31, 2022 •

edited

Loading

tzssangglass Apr 2, 2022 •

edited

Loading