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

feat: add multiple listener support #413

Closed
wants to merge 1 commit into from

Conversation

leki75
Copy link
Contributor

@leki75 leki75 commented Oct 17, 2022

1. Are you opening this pull request for bug-fixes, optimizations or new feature?

New feature

2. Please describe how these code changes achieve your intention.

This PR will add multi-listener support. The user can specify multiple IP:port pairs separated by commas.

3. Please link to the relevant issues (if any).

4. Which documentation changes (if any) need to be made/updated because of this PR?

The gnet.Run command will accept multiple IP:port pairs like this:

gnet.Run(echo, "tcp://192.168.0.100:8000,192.168.0.100:8001,192.168.0.100:8002")

4. Checklist

  • I have squashed all insignificant commits.
  • I have commented my code for explaining package types, values, functions, and non-obvious lines.
  • I have written unit tests and verified that all tests passes (if needed).
  • I have documented feature info on the README (only when this PR is adding a new feature).
  • (optional) I am willing to help maintain this change if there are issues with it later.

if ln, err = initListener(network, addr, options); err != nil {
return
listeners := make(map[int]*listener)
for _, addr := range strings.Split(addrs, ",") {
Copy link
Owner

@panjf2000 panjf2000 Oct 18, 2022

Choose a reason for hiding this comment

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

I think we should update parseProtoAddr() to support string format: "tcp://192.168.0.100:8000,tcp://192.168.0.100:8001,tcp://192.168.0.100:8002" to maintain compatibility when gnet supports multiple listener with different protocols in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, thanks. I will try to fix it and add appropriate tests.

Copy link
Owner

@panjf2000 panjf2000 left a comment

Choose a reason for hiding this comment

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

Thanks for this feature, please add some tests for it.

@misterquestions
Copy link

Wondering if this supports binding multiple protocols at one, for ex. Mixing udp and tcp. Also, is there any news on this?

@gnolizuh
Copy link

Is there a planned time to accept this PR?

@panjf2000
Copy link
Owner

Is there a planned time to accept this PR?

This PR is still in WIP status, I'd love to review it once @leki75 finishes it.

@panjf2000
Copy link
Owner

Move to #578

@panjf2000 panjf2000 closed this Apr 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants