Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: let etcd v3 read_watch handle the case where a chunk contains partial event or multiple events #154

Merged
merged 14 commits into from
Mar 3, 2022
Prev Previous commit
Next Next commit
reformat
  • Loading branch information
nic-6443 committed Mar 2, 2022
commit f428a265cb9c199377d77585e0c686a85b79df3f
6 changes: 3 additions & 3 deletions lib/resty/etcd/v3.lua
Original file line number Diff line number Diff line change
Expand Up @@ -703,9 +703,9 @@ local function request_chunk(self, method, path, opts, timeout)
if not body then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look like this branch is not executed, as body has default value ""?

Copy link
Contributor Author

@nic-6443 nic-6443 Mar 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, my fault, body should initial with nil.

body = chunk
else
-- this branch will only be executed in rare cases,
-- for example, a single event json is larger than the proxy_buffer_size of nginx which proxies etcd,
-- so It would be ok to use a string concat directly without worry about the performance.
-- this branch will only be executed in rare cases, for example, a single event json
-- is larger than the proxy_buffer_size of nginx which proxies etcd, so it would be
-- ok to use a string concat directly without worry about the performance.
body = body .. chunk
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use table.concat to avoid creating too much temp str?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

end

Expand Down