feat(protocol): send worker info (id and pid) to broker #54

bungle · 2024-05-21T16:40:37Z

Summary

This is a feature that sends worker info to broker. This feature is needed for future PRs to implement worker id based broker queues (to replace worker connection based queues).

github-actions · 2024-05-21T16:41:00Z

Luacheck Report

1 tests 0 ✅ 0s ⏱️
1 suites 0 💤
1 files 1 ❌

For more details on these failures, see this check.

Results for commit e649ea0.

♻️ This comment has been updated with latest results.

outsinre

We can postpone this as a standalone job.

lualib/resty/events/protocol.lua

bungle · 2024-05-22T06:07:07Z

We can postpone this as a standalone job.

What does postponing as a standalone job mean? Do you mean we can cut a release without it? I completely agree. This PR just builds basis for future improvements that I will open ”soon”.

chronolaw · 2024-05-23T01:37:55Z

If it is not urgent, I tend to turn it to draft.

lualib/resty/events/protocol.lua

chronolaw · 2024-05-29T00:41:04Z

Question: what is the advantage of replacing connection with worker id? Could I get some explain?

bungle · 2024-05-31T16:49:51Z

Question: what is the advantage of replacing connection with worker id? Could I get some explain?

Workers can crash. So this code:

self._clients[worker_connection]:pop()

if it was something like:

QUEUES[worker_id]:pop()

We could make queues survive across worker crashes (and continue writing on them). We could also simplify a lot of loops to just:

for i=1, WORKER_COUNT do
end

I think that with this library we can and probably should assume that each worker connects to a broker.

lualib/resty/events/protocol.lua

t/protocol.t

tzssangglass

LGTM

### Summary This is a feature that sends worker info to broker. This feature is needed for future PRs to implement worker id based broker queues (to replace worker connection based queues).

### Summary - [style(lib/compat): update module version](#57) - [tests(*): use resty.events.new correctly as documented](#59) - [feat(protocol): send worker info (id and pid) to broker](#54) - [fix(*): option validation of broker id](#62) - [fix(broker): worker id based queues](#60) - [chore(worker): actively close connection to broker on error](#63) - [fix(*): retain events on send failures](#61)

### Summary - [style(lib/compat): update module version](Kong/lua-resty-events#57) - [tests(*): use resty.events.new correctly as documented](Kong/lua-resty-events#59) - [feat(protocol): send worker info (id and pid) to broker](Kong/lua-resty-events#54) - [fix(*): option validation of broker id](Kong/lua-resty-events#62) - [fix(broker): worker id based queues](Kong/lua-resty-events#60) - [chore(worker): actively close connection to broker on error](Kong/lua-resty-events#63) - [fix(*): retain events on send failures](Kong/lua-resty-events#61) Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>

- [style(lib/compat): update module version](Kong/lua-resty-events#57) - [tests(*): use resty.events.new correctly as documented](Kong/lua-resty-events#59) - [feat(protocol): send worker info (id and pid) to broker](Kong/lua-resty-events#54) - [fix(*): option validation of broker id](Kong/lua-resty-events#62) - [fix(broker): worker id based queues](Kong/lua-resty-events#60) - [chore(worker): actively close connection to broker on error](Kong/lua-resty-events#63) - [fix(*): retain events on send failures](Kong/lua-resty-events#61) Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>

bungle force-pushed the feat/send-worker-id-to-broker branch 3 times, most recently from 400ffaa to 6126c0d Compare May 21, 2024 18:11

outsinre reviewed May 22, 2024

View reviewed changes

lualib/resty/events/protocol.lua Outdated Show resolved Hide resolved

bungle force-pushed the feat/send-worker-id-to-broker branch from 6126c0d to afb1020 Compare May 22, 2024 12:46

bungle changed the title ~~feat(protocol): send worker id to broker~~ feat(protocol): send worker info (id and pid) to broker May 22, 2024

chronolaw marked this pull request as draft May 23, 2024 01:37

bungle force-pushed the feat/send-worker-id-to-broker branch from afb1020 to 65b5845 Compare May 28, 2024 12:06

bungle marked this pull request as ready for review May 28, 2024 12:06

bungle force-pushed the feat/send-worker-id-to-broker branch 2 times, most recently from 0a9f2bb to 184c5bb Compare May 28, 2024 12:14

chronolaw reviewed May 28, 2024

View reviewed changes

lualib/resty/events/protocol.lua Outdated Show resolved Hide resolved

bungle force-pushed the feat/send-worker-id-to-broker branch 3 times, most recently from 1dd8b26 to 8a7141e Compare May 28, 2024 14:02

chronolaw previously approved these changes Jun 3, 2024

View reviewed changes

chronolaw requested a review from outsinre June 3, 2024 06:23

outsinre previously approved these changes Jun 6, 2024

View reviewed changes

lualib/resty/events/protocol.lua Outdated Show resolved Hide resolved

outsinre reviewed Jun 6, 2024

View reviewed changes

lualib/resty/events/protocol.lua Show resolved Hide resolved

bungle dismissed stale reviews from outsinre and chronolaw via 62bf45c June 6, 2024 07:57

bungle force-pushed the feat/send-worker-id-to-broker branch from 8a7141e to 62bf45c Compare June 6, 2024 07:57

chronolaw reviewed Jun 6, 2024

View reviewed changes

lualib/resty/events/protocol.lua Show resolved Hide resolved

t/protocol.t Show resolved Hide resolved

tzssangglass previously approved these changes Jun 13, 2024

View reviewed changes

bungle force-pushed the feat/send-worker-id-to-broker branch from 62bf45c to f7df382 Compare June 18, 2024 15:45

gszr approved these changes Jun 18, 2024

View reviewed changes

bungle dismissed tzssangglass’s stale review via f7df382 June 18, 2024 17:57

feat(protocol): send worker id to broker

e649ea0

### Summary This is a feature that sends worker info to broker. This feature is needed for future PRs to implement worker id based broker queues (to replace worker connection based queues).

bungle force-pushed the feat/send-worker-id-to-broker branch from f7df382 to e649ea0 Compare June 18, 2024 17:59

gszr requested a review from tzssangglass June 18, 2024 18:03

tzssangglass approved these changes Jun 18, 2024

View reviewed changes

gszr merged commit 459f8db into main Jun 18, 2024
13 checks passed

gszr deleted the feat/send-worker-id-to-broker branch June 18, 2024 18:05

bungle mentioned this pull request Jun 19, 2024

chore(*): release 0.3.0 #64

Merged

bungle mentioned this pull request Jun 19, 2024

chore(deps): bump lua-resty-events from 0.2.1 to 0.3.0 Kong/kong#13228

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(protocol): send worker info (id and pid) to broker #54

feat(protocol): send worker info (id and pid) to broker #54

bungle commented May 21, 2024 •

edited

Loading

github-actions bot commented May 21, 2024 •

edited

Loading

outsinre left a comment

bungle commented May 22, 2024

chronolaw commented May 23, 2024 •

edited

Loading

chronolaw commented May 29, 2024

bungle commented May 31, 2024

tzssangglass left a comment

feat(protocol): send worker info (id and pid) to broker #54

feat(protocol): send worker info (id and pid) to broker #54

Conversation

bungle commented May 21, 2024 • edited Loading

Summary

github-actions bot commented May 21, 2024 • edited Loading

Luacheck Report

outsinre left a comment

Choose a reason for hiding this comment

bungle commented May 22, 2024

chronolaw commented May 23, 2024 • edited Loading

chronolaw commented May 29, 2024

bungle commented May 31, 2024

tzssangglass left a comment

Choose a reason for hiding this comment

bungle commented May 21, 2024 •

edited

Loading

github-actions bot commented May 21, 2024 •

edited

Loading

chronolaw commented May 23, 2024 •

edited

Loading