refactor(core) integrate new events library #8890

chronolaw · 2022-06-02T06:59:05Z

Summary

Now we have a new event libaray, let's try to introduce it to replace lua-resty-worker-events.

#8715 has too many changes, and it is hard to rebase to master, so I create this new PR.

Full changelog

Modify init_worker_events in global.lua, switch to new events library.
Modify templates/nginx_kong.lua, add unix listening
Modify templates/kong_defaults.lua, add config legacy_worker_events
Add a temporary kong/resty/healthcheck.lua to use resty.events.compat
Modify test cases to pass ci, wrap them with wait_until()
events library version is 0.1.0

Notice

Temporary kong/resty/healthcheck.lua should be replaced by a formal version.
Flaky test 02-integration/05-proxy/10-balancer/01-healthchecks_spec.lua now is fixed.

chronolaw · 2022-06-02T07:00:43Z

kong/runloop/balancer/healthcheckers.lua

@@ -20,7 +20,9 @@ local healthcheckers_M = {}
 local healthcheck_subscribers = {}

 function healthcheckers_M.init()
-  healthcheck = require("resty.healthcheck") -- delayed initialization
+  -- XXX: test only
+  healthcheck = require("kong.resty.healthcheck") -- delayed initialization


Change it before merge to master.

We will remove this temporary file after feature freeze.

.requirements

bungle

I have approved this with the comments. I still wish we could:

drop the old events library completely
remove the compatibility layer and use the new apis

kong/global.lua

.requirements

.github/workflows/build_and_test.yml

chronolaw · 2022-06-02T09:37:00Z

I have approved this with the comments. I still wish we could:

drop the old events library completely

remove the compatibility layer and use the new apis

It is a bit risky to switch to the new events library, so we have to keep the old one.

If the new events library is proven working well, we can remove old one.

bungle · 2022-06-02T11:44:32Z

It is a bit risky to switch to the new events library, so we have to keep the old one.

I know. It is risky to update events library, router and timer. But if on all of these we carry to old code with us, it means we have huge amount of different ways to configure Kong and each bug gets harder to resolve as you need to find out:

what router were you using
what events library were you using
was the timer library being used

Plus, we already have done a lot of deprecation removals. I do know that this is not the right place to decide on it, but I still would love to have the last conversation on it. Having wrpc and legacy protocol feels similar, now we need to maintain two implementations. This will definitely slow us down and create unique bugs.

This is a very big risk too. And it possibly means we need to carry the code around. For events, I would be more than willing to take that risk. If people report issues on 3.0.0 we will fix them in 3.0.1, etc. What I mean is that I don't want this to become a theme, where we reimplement something, and then don't remove the older one.

If the new events library is proven working well, we can remove old one.

Is there any reason to believe it would not? I mean we definitely have bugs in current events library too.

chronolaw · 2022-06-02T12:36:18Z

I understood your feeling.

The problem is that there are many places to use events library with old apis, we will cost a lot of time to find/change/test them . I think it can not be done in this PR.

If this PR is merged maybe we can start a new PR to clean old apis.

bungle · 2022-06-02T17:53:14Z

The problem is that there are many places to use events library with old apis, we will cost a lot of time to find/change/test them . I think it can not be done in this PR.

Yes, we can make separate PR to remove compatibility mode, if decided so. But perhaps we need to first decide. I try to write something.

flrgh · 2022-06-04T00:34:58Z

spec/02-integration/05-proxy/10-balancer/03-consistent-hashing_spec.lua

-          }))
+
+          local res
+          helpers.wait_until(function()


Seeing this pattern added in so many integration tests is concerning to me, because:

helpers.wait_until is generic and doesn't really provide meaningful output when it fails, so it's harder to debug failing tests

it makes me worry that there are many more test cases which may be flaky, and we could be chasing after them for a long time

Is there no alternative to this? Is there any function within the new events library to block until all workers have handled/seen events up until some known event? If so, we could expose it via the admin API and incorporate it into spec.helpers. Alternatively, we could add an optional, (maybe private) query arg to the CRUD APIs for traditional mode, so that we can do things like POST /services?sync=true, and it would block until all workers have caught up with events.

Additionally, something that might help would be to set nginx_main_worker_processes to 1 in most cases, since this will cut down on how much time is spent ensuring all workers are in sync.

The new events library affects too many test cases, It is a hard work to make all of them green.
I think if we could improve these test cases after this PR is merged.

.requirements

chronolaw added 30 commits March 31, 2022 20:55

events frame/protocol

e241f6d

callback.lua

b6afdb5

broker.lua

2b5c72b

client.lua

aead4cf

client.lua

757b533

default opts for broker

619b07d

post local event

30be16b

code clean for queue

7576479

poll simulate

1f76e6c

clean broker

2ac81a4

clean worker

3752d72

change worker events in global.lua

06e281f

add worker events server in template

9e7576a

add event config

47fc24b

nginx kong template

d660e81

add events code into rockspec

e09b7f8

code clean

6139960

code clean

ba458db

add config entry

2e32759

localize assert

18f5143

move events code to another repo

0a629fe

fix broker in template

b9b6dd0

try run github ci

5dd74ce

stream supports events

606021d

use unified resty.events

ca3a618

change config params

997ec27

add resty events in requirements

489e184

fix rebase error

4280adc

conf events mechanism

e8b8468

edit custom_nginx.template

360c0b5

chronolaw commented Jun 2, 2022

View reviewed changes

chronolaw mentioned this pull request Jun 2, 2022

refactor(core) using new events library #8715

Closed

chronolaw added this to the 3.0 milestone Jun 2, 2022

chronolaw commented Jun 2, 2022

View reviewed changes

.requirements Outdated Show resolved Hide resolved

bungle approved these changes Jun 2, 2022

View reviewed changes

kong/global.lua Show resolved Hide resolved

.requirements Outdated Show resolved Hide resolved

.github/workflows/build_and_test.yml Outdated Show resolved Hide resolved

flrgh reviewed Jun 4, 2022

View reviewed changes

chronolaw added pr/do not merge and removed pr/do not merge labels Jun 6, 2022

use master branch of build tool

afceb2e

github-actions bot removed the chore Not part of the core functionality of kong, but still needed label Jun 6, 2022

chronolaw added 8 commits June 6, 2022 12:05

fix 05-proxy/02-router_spec.lua

a829d5a

KONG_BUILD_TOOLS = master

17bebb0

clean some comments

e40f663

build tools use master

5306838

clean 01-admin_api_spec.lua

1d49d7a

clean 01-admin_api_spec.lua

ff3f751

clean 02-start_stop_spec.lua

a424b97

fix merge conflict

4a7baca

chronolaw commented Jun 7, 2022

View reviewed changes

.requirements Outdated Show resolved Hide resolved

chronolaw added 2 commits June 7, 2022 14:55

events release 0.1.0

f1d4330

bump build-tools to 4.30.0

e830adb

chronolaw mentioned this pull request Jun 7, 2022

feat(healthcheck) support setting the events module Kong/lua-resty-healthcheck#105

Merged

Merge branch 'master' into feat/integrate_new_events

dcf8fd6

fffonion merged commit 740bad7 into master Jun 8, 2022

fffonion deleted the feat/integrate_new_events branch June 8, 2022 07:07

chobits mentioned this pull request Nov 16, 2023

kong reload , a few hours later, cpu 100% ( luajit table rehash/resize by stapxx profiling),help... #11877

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(core) integrate new events library #8890

refactor(core) integrate new events library #8890

chronolaw commented Jun 2, 2022 •

edited

Loading

chronolaw Jun 2, 2022

chronolaw Jun 7, 2022

bungle left a comment •

edited

Loading

chronolaw commented Jun 2, 2022

bungle commented Jun 2, 2022

chronolaw commented Jun 2, 2022

bungle commented Jun 2, 2022

flrgh Jun 4, 2022

chronolaw Jun 6, 2022

refactor(core) integrate new events library #8890

refactor(core) integrate new events library #8890

Conversation

chronolaw commented Jun 2, 2022 • edited Loading

Summary

Full changelog

Notice

chronolaw Jun 2, 2022

Choose a reason for hiding this comment

chronolaw Jun 7, 2022

Choose a reason for hiding this comment

bungle left a comment • edited Loading

Choose a reason for hiding this comment

chronolaw commented Jun 2, 2022

bungle commented Jun 2, 2022

chronolaw commented Jun 2, 2022

bungle commented Jun 2, 2022

flrgh Jun 4, 2022

Choose a reason for hiding this comment

chronolaw Jun 6, 2022

Choose a reason for hiding this comment

chronolaw commented Jun 2, 2022 •

edited

Loading

bungle left a comment •

edited

Loading