feat(lib): increase max payload to at most 16MB #37

chronolaw · 2023-06-26T11:02:50Z

FTI-4963
KAG-1899

This PR will replace the PR Kong/kong#11117.

chronolaw · 2023-06-26T22:53:03Z

lualib/resty/events/frame.lua

@@ -82,12 +89,17 @@ function _M.send(sock, payload)
        return nil, err
    end

-    local bytes, err = sock:send(uint16_to_bytes(payload_len) .. payload)
+    local data = SEND_DATA


Here I used tablepool but remove it now.

I think that we won't get race condition, am I right?

Can we just not to use the upvalue as local data = SEND_DATA and make the data structure to be sent directly, like:

local bytes, err = sock:send({ uint_to_bytes(payload_len), payload })

I'm afraid I don't understand why you tend to use the upvalue here.

It will create too many temporary small tables, increase the burden of gc.

chronolaw · 2023-07-11T01:11:16Z

Reopen if needed.

windmgc

LGTM

ADD-SP · 2023-07-12T06:29:14Z

lualib/resty/events/frame.lua

+    data[1] = uint_to_bytes(payload_len)
+    data[2] = payload
+
+    local bytes, err = sock:send(data)


For the current implementation of lua-nginx-module, I think there is no race condition on SEND_DATA.

Just mention that changing data during the call of sock:send() is undefined behavior, but I think it is not a blocker of this PR.

OpenAPI spec in DevPortal or updating a big config entry for plugins, they can not work as expected. With the new parameter, the user can decide the payload size to meet their needs. In this PR, a new parameter, `worker_events_max_payload` is added, which allows to specify the payload size the `worker_events` lib can accept. The default size is 64k, and the max allowed to set is 16M Bytes. The corresponding PR for `worker_events` lib is [#37](Kong/lua-resty-events#37) FTI-4963

#11214) * With a hard-coded payload size, for some use cases like uploading a big OpenAPI spec in DevPortal or updating a big config entry for plugins, they can not work as expected. With the new parameter, the user can decide the payload size to meet their needs. In this PR, a new parameter, `worker_events_max_payload` is added, which allows to specify the payload size the `worker_events` lib can accept. The default size is 64k, and the max allowed to set is 16M Bytes. The corresponding PR for `worker_events` lib is [#37](Kong/lua-resty-events#37) FTI-4963 * add changelog entry * Update kong.conf.default Co-authored-by: Datong Sun <datong.sun@konghq.com> * add test case and bump lua-resty-events * correct the default value, and add an entry for bumping the version of lua-resty-events * 1. append PR number to the changelog entry of lua-resty-events 2. correct the spec test 3. style * Update CHANGELOG.md --------- Co-authored-by: Datong Sun <datong.sun@konghq.com> Co-authored-by: Chrono <chrono_cpp@me.com>

#11214) * With a hard-coded payload size, for some use cases like uploading a big OpenAPI spec in DevPortal or updating a big config entry for plugins, they can not work as expected. With the new parameter, the user can decide the payload size to meet their needs. In this PR, a new parameter, `worker_events_max_payload` is added, which allows to specify the payload size the `worker_events` lib can accept. The default size is 64k, and the max allowed to set is 16M Bytes. The corresponding PR for `worker_events` lib is [#37](Kong/lua-resty-events#37) FTI-4963 * add changelog entry * Update kong.conf.default Co-authored-by: Datong Sun <datong.sun@konghq.com> * add test case and bump lua-resty-events * correct the default value, and add an entry for bumping the version of lua-resty-events * 1. append PR number to the changelog entry of lua-resty-events 2. correct the spec test 3. style * Update CHANGELOG.md --------- Co-authored-by: Datong Sun <datong.sun@konghq.com> Co-authored-by: Chrono <chrono_cpp@me.com> (cherry picked from commit ff59edb)

#11214) * With a hard-coded payload size, for some use cases like uploading a big OpenAPI spec in DevPortal or updating a big config entry for plugins, they can not work as expected. With the new parameter, the user can decide the payload size to meet their needs. In this PR, a new parameter, `worker_events_max_payload` is added, which allows to specify the payload size the `worker_events` lib can accept. The default size is 64k, and the max allowed to set is 16M Bytes. The corresponding PR for `worker_events` lib is [#37](Kong/lua-resty-events#37) FTI-4963 * add changelog entry * Update kong.conf.default Co-authored-by: Datong Sun <datong.sun@konghq.com> * add test case and bump lua-resty-events * correct the default value, and add an entry for bumping the version of lua-resty-events * 1. append PR number to the changelog entry of lua-resty-events 2. correct the spec test 3. style * Update CHANGELOG.md --------- Co-authored-by: Datong Sun <datong.sun@konghq.com> Co-authored-by: Chrono <chrono_cpp@me.com>

#11214) * With a hard-coded payload size, for some use cases like uploading a big OpenAPI spec in DevPortal or updating a big config entry for plugins, they can not work as expected. With the new parameter, the user can decide the payload size to meet their needs. In this PR, a new parameter, `worker_events_max_payload` is added, which allows to specify the payload size the `worker_events` lib can accept. The default size is 64k, and the max allowed to set is 16M Bytes. The corresponding PR for `worker_events` lib is [#37](Kong/lua-resty-events#37) FTI-4963 * add changelog entry * Update kong.conf.default Co-authored-by: Datong Sun <datong.sun@konghq.com> * add test case and bump lua-resty-events * correct the default value, and add an entry for bumping the version of lua-resty-events * 1. append PR number to the changelog entry of lua-resty-events 2. correct the spec test 3. style * Update CHANGELOG.md --------- Co-authored-by: Datong Sun <datong.sun@konghq.com> Co-authored-by: Chrono <chrono_cpp@me.com> (cherry picked from commit ff59edb)

chronolaw added 4 commits June 26, 2023 18:12

increase max payload

2158f5a

remove limitation test case

fc7da48

readme

e796df1

bump to 0.2.0

b423c13

chronolaw mentioned this pull request Jun 26, 2023

fix(worker_event): send large payload Kong/kong#11117

Closed

3 tasks

chronolaw marked this pull request as ready for review June 26, 2023 11:36

chronolaw changed the title ~~feat(frame): increase max payload~~ feat(frame): increase max payload to 16MB Jun 26, 2023

chronolaw added 6 commits June 26, 2023 19:50

test 64kb data

6a1f942

optimize sock:send

3959459

tablepool

02beaa4

test send 1mb

2d34df3

tablepool_release

1564c16

remove tablepool

7e5d115

chronolaw commented Jun 26, 2023

View reviewed changes

tests

f3f7dd5

chronolaw closed this Jul 11, 2023

chronolaw reopened this Jul 11, 2023

windmgc approved these changes Jul 11, 2023

View reviewed changes

bytes_to_uint

70cc83f

chronolaw changed the title ~~feat(frame): increase max payload to 16MB~~ feat(frame): increase max payload to at most 16MB Jul 12, 2023

opt max_payload_len

7b00d72

chronolaw force-pushed the feat/increase_size_of_message branch from 17322ea to 7b00d72 Compare July 12, 2023 03:53

chronolaw added 4 commits July 12, 2023 11:57

readme

002cd50

init param check

595229b

worker.lua

41f342d

style fix

d0a0e8c

chronolaw force-pushed the feat/increase_size_of_message branch from 14aac25 to d0a0e8c Compare July 12, 2023 04:10

chronolaw added 2 commits July 12, 2023 12:27

more tests

0538fe0

tests

57147eb

ADD-SP approved these changes Jul 12, 2023

View reviewed changes

chronolaw changed the title ~~feat(frame): increase max payload to at most 16MB~~ feat(lib): increase max payload to at most 16MB Jul 12, 2023

liverpool8056 approved these changes Jul 12, 2023

View reviewed changes

chronolaw merged commit 8448a92 into main Jul 12, 2023

chronolaw deleted the feat/increase_size_of_message branch July 12, 2023 07:13

liverpool8056 mentioned this pull request Jul 12, 2023

fix(core/worker_events): overwrite the post method of worker_events Kong/kong#10862

Closed

3 tasks

liverpool8056 mentioned this pull request Jul 12, 2023

fix(core): Add a new parameter worker_event_max_payload to kong.conf Kong/kong#11214

Merged

3 tasks

This was referenced Jul 21, 2023

[cherry]fix(core): Add a new parameter worker_event_max_payload to kong.con… Kong/kong#11257

Closed

[cherry]fix(core): Add a new parameter worker_event_max_payload to kong.con… Kong/kong#11258

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(lib): increase max payload to at most 16MB #37

feat(lib): increase max payload to at most 16MB #37

chronolaw commented Jun 26, 2023 •

edited

Loading

chronolaw Jun 26, 2023

liverpool8056 Jul 11, 2023 •

edited

Loading

chronolaw Jul 11, 2023

chronolaw commented Jul 11, 2023

windmgc left a comment

ADD-SP Jul 12, 2023

feat(lib): increase max payload to at most 16MB #37

feat(lib): increase max payload to at most 16MB #37

Conversation

chronolaw commented Jun 26, 2023 • edited Loading

chronolaw Jun 26, 2023

Choose a reason for hiding this comment

liverpool8056 Jul 11, 2023 • edited Loading

Choose a reason for hiding this comment

chronolaw Jul 11, 2023

Choose a reason for hiding this comment

chronolaw commented Jul 11, 2023

windmgc left a comment

Choose a reason for hiding this comment

ADD-SP Jul 12, 2023

Choose a reason for hiding this comment

chronolaw commented Jun 26, 2023 •

edited

Loading

liverpool8056 Jul 11, 2023 •

edited

Loading