Skip to content

feat(disvocery): Add zookeeper discovery #12772

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
Open
dongjiang1989 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat(disvocery): Add zookeeper discovery #12772

dongjiang1989 wants to merge 2 commits into from

Conversation

@dongjiang1989
Copy link
Contributor

@dongjiang1989 dongjiang1989 commented Nov 24, 2025

Description

Which issue(s) this PR fixes:

Fixes #3231 #6687

Add zookeeper discovery

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)

@dongjiang1989 dongjiang1989 changed the title feat: Add zookeeper discovery feat(disvocery): Add zookeeper discovery Nov 24, 2025
@dongjiang1989 dongjiang1989 force-pushed the add-zookeeper-discovery branch from 7bfba46 to 01f70df Compare November 24, 2025 11:43
@dongjiang1989 dongjiang1989 marked this pull request as ready for review November 24, 2025 12:29
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Nov 24, 2025
Baoyuantop
Baoyuantop reviewed Nov 25, 2025
View reviewed changes
Copy link
Contributor

@Baoyuantop Baoyuantop left a comment

Choose a reason for hiding this comment

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

Hi @dongjiang1989, thanks for your contribution. Could you add documentation and testing for this feature? You can refer to other service discovery modules for reference.

@dongjiang1989
Copy link
Contributor Author

dongjiang1989 commented Nov 25, 2025

Hi @dongjiang1989, thanks for your contribution. Could you add documentation and testing for this feature? You can refer to other service discovery modules for reference.

Thanks @Baoyuantop
Documentation and Testing will coming

@dongjiang1989 dongjiang1989 force-pushed the add-zookeeper-discovery branch 3 times, most recently from 3993d37 to ea3c33e Compare November 26, 2025 02:24
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Nov 26, 2025
@dongjiang1989 dongjiang1989 force-pushed the add-zookeeper-discovery branch from f25602f to 3435db2 Compare November 26, 2025 03:22
@dongjiang1989
Copy link
Contributor Author

dongjiang1989 commented Nov 26, 2025

@Baoyuantop Please review docs, thanks

@Baoyuantop
Copy link
Contributor

Baoyuantop commented Dec 3, 2025

Hi @dongjiang1989, additional tests are needed to verify that the function works correctly.

Baoyuantop
Baoyuantop reviewed Dec 3, 2025
View reviewed changes
@dongjiang1989
Copy link
Contributor Author

dongjiang1989 commented Dec 3, 2025

Hi @dongjiang1989, additional tests are needed to verify that the function works correctly.

Got it. I'll add UT soon

@Baoyuantop Baoyuantop added the wait for update wait for the author's response in this issue/PR label Dec 8, 2025
@Baoyuantop
Copy link
Contributor

Baoyuantop commented Dec 16, 2025

Hi @dongjiang1989, any update?

@dongjiang1989 dongjiang1989 force-pushed the add-zookeeper-discovery branch from 9ce82a8 to 836a97b Compare December 17, 2025 10:47
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Dec 17, 2025
@dongjiang1989 dongjiang1989 force-pushed the add-zookeeper-discovery branch from 448b4c1 to f81575e Compare December 18, 2025 03:28
@dongjiang1989
add zookeeper discovery
00618b0 
Signed-off-by: dongjiang1989 <[email protected]>
@dongjiang1989 dongjiang1989 force-pushed the add-zookeeper-discovery branch from 80c05ef to 00618b0 Compare December 18, 2025 10:03
@dongjiang1989
Copy link
Contributor Author

dongjiang1989 commented Dec 18, 2025

Hi @dongjiang1989, any update?

Thanks @Baoyuantop
Update. Please re-check it

Baoyuantop
Baoyuantop reviewed Dec 19, 2025
View reviewed changes
apisix/discovery/zookeeper/init.lua
end

-- Scheduled fetch of all service instances (full cache update)）
local function fetch_all_services()
Copy link
Contributor

@Baoyuantop Baoyuantop Dec 19, 2025

Choose a reason for hiding this comment

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

Do not fetch all services from ZooKeeper. This can cause significant network overhead when there are many services in ZooKeeper. It is recommended to refer to the Nacos implementation and use a timer to iterate through the currently configured Upstream, Route, and Service resources of APISIX, collecting all currently used service_names (with discovery_type set to zookeeper). Only perform ZooKeeper queries and cache updates on these "in-use" services.

apisix/discovery/zookeeper/init.lua
log.info("zookeeper_conf:", core.json.encode(zookeeper_conf))
-- Start the timer
if not fetch_timer then
fetch_timer = ngx.timer.every(zookeeper_conf.fetch_interval, fetch_all_services)
Copy link
Contributor

@Baoyuantop Baoyuantop Dec 19, 2025

Choose a reason for hiding this comment

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

It is recommended that only privileged processes request data from ZooKeeper and write it to the shared dict, while other processes read the data from the shared dict.

update by codereview
c2a123c 
Signed-off-by: jiangdong <[email protected]>
@Baoyuantop
Copy link
Contributor

Baoyuantop commented Dec 24, 2025

Hi @dongjiang1989, following up on the previous review comments. Please let us know if you have any updates. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Baoyuantop Baoyuantop Baoyuantop left review comments

At least 3 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

enhancement New feature or request size:XXL This PR changes 1000+ lines, ignoring generated files. wait for update wait for the author's response in this issue/PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

request help: how to use apisix with zookeeper service discovery

2 participants

@dongjiang1989 @Baoyuantop