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.

Sign up for GitHub

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

Jump to bottom

fix(etcd): reuse cli and enable keepalive #9420

Merged
merged 4 commits into from
May 17, 2023
Merged

fix(etcd): reuse cli and enable keepalive #9420

merged 4 commits into from
May 17, 2023

Conversation

kingluo
Copy link
Contributor

@kingluo kingluo commented May 6, 2023

Description

  1. The JWT auth token is stored in the etcd cli table, so reuse the table between calls to avoid repeat auth calls.
  2. The path between conf server and etcd should enable keepalive so that we could reuse the same connection to do etcd calls. Besides avoiding connection overhead, the etcd v3 uses connection based auth.

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)

@kingluo kingluo marked this pull request as ready for review May 11, 2023 16:24
kingluo
kingluo commented May 11, 2023
View reviewed changes
t/core/config_etcd.t Show resolved Hide resolved
t/core/etcd.t
content_by_lua_block {
local etcd_lib = require("resty.etcd")
-- the mock won't take effect when using gRPC because the connection will be cached
etcd_lib.new = function()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

hijacking resty.etcd.new does not work after the cli is created once, because it's reused afterwards.

leslie-tsang
leslie-tsang previously approved these changes May 16, 2023
View reviewed changes
@leslie-tsang leslie-tsang self-requested a review May 17, 2023 01:05
@monkeyDluffy6017 monkeyDluffy6017 merged commit 68287e9 into apache:master May 17, 2023
AlinsRan pushed a commit to AlinsRan/apisix that referenced this pull request Jul 7, 2023
@kingluo @AlinsRan
fix(etcd): reuse cli and enable keepalive (apache#9420)
80e0482
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@monkeyDluffy6017 monkeyDluffy6017 monkeyDluffy6017 approved these changes

@leslie-tsang leslie-tsang leslie-tsang approved these changes

@soulbird soulbird Awaiting requested review from soulbird

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kingluo @monkeyDluffy6017 @leslie-tsang