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

--socket-reuse-port and --socket-reuse-address These two flags have bugs #16399

Closed
coderchuan opened this issue Aug 10, 2023 · 8 comments
Closed

Comments

@coderchuan
Copy link

coderchuan commented Aug 10, 2023

Bug report

What happened?

--socket-reuse-port and --socket-reuse-address invalid in the configuration file, but are valid in the command flag

What did you expect to happen?

--socket-reuse-port and --socket-reuse-address in the command flags should behave the same as in the configuration file

Detail

Following are the command flags,both node1 and node2 listening to the same port are running normally, even though they use the same port, they work because of the --socket-reuse-port flag

  1. etcd --name=node1 --force-new-cluster=false --initial-advertise-peer-urls=http://localhost:2380 --listen-peer-urls="http://localhost:2380" --advertise-client-urls="http://localhost:2379" --listen-client-urls="http://localhost:2379" --socket-reuse-port=true
  2. etcd --name=node2 --force-new-cluster=false --initial-advertise-peer-urls=http://localhost:2380 --listen-peer-urls="http://localhost:2380" --advertise-client-urls="http://localhost:2379" --listen-client-urls="http://localhost:2379" --socket-reuse-port=true

The following is the content of the configuration file in yaml format, running the first one is normal, running the second one will report address already in used, even with the socket-reuse-port flag enabled

  • node1, save as node1.yaml etcd --config-file=node1.yaml
    name: node1
    force-new-cluster: false
    initial-advertise-peer-urls: http://localhost:2380
    listen-peer-urls: "http://localhost:2380"
    advertise-client-urls: "http://localhost:2379"
    listen-client-urls: "http://localhost:2379"
    socket-reuse-port: true
  • node2, save as node2.yaml etcd --config-file=node2.yaml
    name: node2
    force-new-cluster: false
    initial-advertise-peer-urls: http://localhost:2380
    listen-peer-urls: "http://localhost:2380"
    advertise-client-urls: "http://localhost:2379"
    listen-client-urls: "http://localhost:2379"
    socket-reuse-port: true

Etcd version

etcd Version: 3.5.9
Git SHA: bdbbde998
Go Version: go1.19.9
Go OS/Arch: linux/amd64
@jmhbnz
Copy link
Member

jmhbnz commented Aug 11, 2023

Thanks for raising this @coderchuan.

After some quick investigation it looks as though the config file might work as follows:

socket-options:
  reuse-port: true

Can you please check and confirm? We may need to follow-up with an improvement to documentation or adjusting the structure so it follows all other configuration values.

Edit: We could perhaps update the config file example to show how socket-options should be used as it is currently silent on it: https://github.com/etcd-io/etcd/blob/main/etcd.conf.yml.sample

@coderchuan
Copy link
Author

Thanks for raising this @coderchuan.

After some quick investigation it looks as though the config file might work as follows:

socket-options:
  reuse-port: true

Can you please check and confirm? We may need to follow-up with an improvement to documentation or adjusting the structure so it follows all other configuration values.

Edit: We could perhaps update the config file example to show how socket-options should be used as it is currently silent on it: https://github.com/etcd-io/etcd/blob/main/etcd.conf.yml.sample

Tested using the following config in the config file still doesn't work

  • node1, save as node1.yaml , etcd --config-file=node1.yaml

    name: node1
    force-new-cluster: false
    initial-advertise-peer-urls: http://localhost:2380
    listen-peer-urls: "http://localhost:2380"
    advertise-client-urls: "http://localhost:2379"
    listen-client-urls: "http://localhost:2379"
    socket-options:  
        reuse-port: true
    • node2, save as node2.yaml , etcd --config-file=node2.yaml
    name: node2
    force-new-cluster: false
    initial-advertise-peer-urls: http://localhost:2380
    listen-peer-urls: "http://localhost:2380"
    advertise-client-urls: "http://localhost:2379"
    listen-client-urls: "http://localhost:2379"
    socket-options:  
        reuse-port: true

@jmhbnz
Copy link
Member

jmhbnz commented Aug 11, 2023

Tested using the following config in the config file still doesn't work

Ahh apologies, I see you are using 3.5.9. The approach I posted does work against main but you're right, not against release-3.5 branch. Let me try and find the difference that fixes it.

Edit: I believe changes from these two commits in main need to be backported to release-3.5:

Please feel free to raise a pull request for this if you have capacity.

@coderchuan
Copy link
Author

coderchuan commented Aug 11, 2023

Tested using the following config in the config file still doesn't work

Ahh apologies, I see you are using 3.5.9. The approach I posted does work against main but you're right, not against release-3.5 branch. Let me try and find the difference that fixes it.

Edit: I believe changes from these two commits in main need to be backported to release-3.5:

Please feel free to raise a pull request for this if you have capacity.

In version 3.6.x the configuration you said does work, thanks. But I think the same option name should be used in both the configuration file and the command flags, and perhaps the same parser should be used regardless of the command flags, configuration file or environment variable, so such problems can be avoided. As far as I know, the options in the configuration file in mysql will be parsed as flags in the command line and the command line options will be used in the final run .

@jmhbnz
Copy link
Member

jmhbnz commented Sep 7, 2023

Hey @coderchuan - The fix to allow socket options to be configured via file has made it's way to our release-3.5 branch in #16435 so will be included in our next release 3.5.10, refer https://github.com/etcd-io/etcd/blob/main/CHANGELOG/CHANGELOG-3.5.md#v3510-tbd.

I tend to agree with you in relation to also aligning the naming for the configuration options so they follow a consistent standard, though please do note that would be a lower priority for now for the etcd project. You can take a look at our current priorities in our roadmap. We certainly welcome new contributors if you do have some time to take a look at raising the fix for the naming alignment.

Thanks again for raising this issue 🙏🏻

@serathius
Copy link
Member

@jmhbnz can this issue be closed with #16399 ?

@jmhbnz
Copy link
Member

jmhbnz commented Oct 17, 2023

@jmhbnz can this issue be closed with #16399 ?

Do you mean with #16733? If so yes, I was planning to close this once 3.5.10 releases.

@serathius
Copy link
Member

We should just close with information that fix will be available in v3.5.10

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

No branches or pull requests

3 participants