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

Allow selecting random port #7652

Open
Phantal opened this issue Aug 12, 2020 · 9 comments
Open

Allow selecting random port #7652

Phantal opened this issue Aug 12, 2020 · 9 comments

Comments

@Phantal
Copy link

Phantal commented Aug 12, 2020

if (server.port != 0 &&

I am requesting that the allowed values for the port be changed as follows:

  • 0..65535 are valid
  • all other values disable TCP listening

0 has special meaning: select an unused port. This port can then be queried with tools like lsof, netstat, ss, etc.

@itamarhaber
Copy link
Member

Hello @Phantal

Thanks for proposing this feature request. The problem with your suggestion is that historically the value of 0 means that Redis will not listen on TCP ports, so if we make the change we'll break the contract with the userbase. This, however, can be easily solved by using a different value, e.g. -1, or even a new config flag.

That said, I'm not totally clear on the use case here. As Redis is a non-discoverable service, in the sense that a remote client needs beforehand knowledge of its address and port, a random choice of port seems unneeded. Please clarify how this can be used in the wild.

@Phantal
Copy link
Author

Phantal commented Aug 12, 2020 via email

@oranagra
Copy link
Member

this could also possibly be a solution to a problem we are / were facing with our test suite.

the code we have now is some loop that tries to find a free port (see find_available_port), then tries to spawn redis that binds on it. if it failed to bind due to a race (someone else using that port first), we try the whole thing again. see this as an example: #7635
in theory letting redis choose the port and then sniffing the port number by some way (e.g. parsing the log file) could be a solution.

@yossigo what do you think

Note that in the non-cluster tests this is not a good solution, since we must have each test thread use a different port range so that in some races in the test teardown servers will not try to replicate from ones in a different test.
And in the cluster tests, there's no concurrency and i simply can't understand why they still fail despite running in a container and having that retry loop.

@yossigo
Copy link
Member

yossigo commented Aug 12, 2020

I am not in favor of this for several reasons:

  1. Dynamic port allocation could make sense if we also had a service discovery framework, so a process binds to some random port and then has a standard way to announce it. Having to poke around to discover what port seems like a very specific hack to me.
  2. Modern environments rely on containers (in this context, network namespaces really) to handle this kind of contention. So I really don't see any justification to create such discovery framework.

@oranagra I think solving the tests issues externally would be anyway simpler and more reliable.

@Phantal Do processes in your environment continuously race each other for resources (TCP ports)? If they don't do that, I guess a simpler approach would be to wrap Redis with some script that handles it:

  • Query netstat to find an unused port
  • Start Redis with that port
  • Retry if failed

The only other potential solution I see is to be support run-time CONFIG SET port (currently the port can only be read from config file on startup).

This could get tricky, in particular now that we have both TCP and TLS, etc.. But it could also be simplified by allowing a port to be set if it was not previously configured. So, you could start with no TCP port configured, then attempt random CONFIG SET port which would only succeed if bind() was successful.

This would have the benefits of (a) allocation is atomic, so it works well in a dynamic environment; (b) it MIGHT be marginally useful in some other cases.

@sergey-komissarov
Copy link

Hello, we are facing the same situation.
We want to start multiple redis servers on the single node with some other services which uses random ports. Actually we have 50-100 services per node.

@yossigo May be it is acceptable to write bound port value in the file specified by the config? It should be enough in most cases.

@yossigo
Copy link
Member

yossigo commented Jan 19, 2021

@sergey-komissarov I'm not sure I understand your suggestion, do you mean specify a port range Redis should allocate from? How do you plan to determine the mapping between ports and instances if it's random?

@sergey-komissarov
Copy link

@yossigo I thought about config or command line option 'port-file' with absolute path to the file where server writes port number as text. Of course file path must be unique for each instance on the node, but this may be handled by node maintaners.

@yossigo
Copy link
Member

yossigo commented Feb 3, 2021

@sergey-komissarov Your proposal is valid, but I think it goes a relatively long way (new config option) to solve a very specific problem.

I am more in favor of simply making port runtime configurable as described above, so the service launcher can start Redis with a Unix socket and then use CONFIG SET port with a random port until successful.

@ThibaultLemaire
Copy link

ThibaultLemaire commented Apr 27, 2023

Hello, just chiming in to say that would be useful to us @BelledonneCommunications as well, because we use redis in our automated test suite.

Both being able to ask for the port to be dynamic (and then e.g. parsing stdout to get it) or being able to CONFIG SET it in a loop at runtime would be satisfactory. (Although my personal preference goes to the first solution. It's simpler, and since it's only for tests, a new version of redis changing its output wouldn't break anything serious.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

No branches or pull requests

6 participants