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

cmd, node: initialize ports with --instance #29268

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

weiihann
Copy link
Contributor

@weiihann weiihann commented Mar 15, 2024

Description

This PR adds a new CLI flag called --instance <value>, used to configure ports when running multiple nodes on the same machine to avoid port conflicts. This is only applicable for port, authrpc.port, discovery,port, http.port, ws.port.

The calculation of port numbers is as follows:

authrpc.port = 8551 (default) + `instance`*100 - 100
http.port = 8545 (default) - `instance` + 1
ws.port = 8546 (default) + `instance`*2 - 2
port = 30303 (default) + `instance` - 1
discovery.port = 30303 (default) + `instance` - 1

In a scenario where the user specify both --instance and the supported port list (i.e. port, authrpc.port, ...), then the ports specified will supersede the port values configured by --instance.

Example

Usage: geth --instance 2

authrpc.port = 8651
http.port = 8544
ws.port = 8548
port = 30304
discovery.port = 30304

Usage: geth --instance 2 --authrpc.port 8551

authrpc.port = 8551
http.port = 8544
ws.port = 8548
port = 30304
discovery.port = 30304

Usage: geth --instance 201

Fatal: Instance number 201 is too high, maximum is 200

Credits

This flag is inspired by reth.

@weiihann weiihann requested a review from fjl as a code owner March 15, 2024 07:27
@weiihann weiihann changed the title V1.13.14/cmd instance cmd, node: initialize ports with --instance Mar 15, 2024
Copy link
Contributor

@jwasinger jwasinger left a comment

Choose a reason for hiding this comment

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

I can see how this flag would be useful when running multiple local nodes on the same host. Because the main use-case for this is for testing, I would opt to not include this flag in Geth.

@@ -114,6 +114,13 @@ var (
Usage: "Minimum free disk space in MB, once reached triggers auto shut down (default = --cache.gc converted to MB, 0 = disabled)",
Category: flags.EthCategory,
}
InstanceFlag = &cli.IntFlag{
Name: "instance",
Usage: "Configures the ports to avoid conflicts when running multiple nodes on the same machine. Maximum is 200. Only applicable for: port, authrpc.port, discovery,port, http.port, ws.port",
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a complete explanation of what this flag does, similarly to the full description from reth:

          Add a new instance of a node.
          
          Configures the ports of the node to avoid conflicts with the defaults. This is useful for running multiple nodes on the same machine.
          
          Max number of instances is 200. It is chosen in a way so that it's not possible to have port numbers that conflict with each other.
          
          Changes to the following port numbers: - DISCOVERY_PORT: default + `instance` - 1 - AUTH_PORT: default + `instance` * 100 - 100 - HTTP_RPC_PORT: default - `instance` + 1 - WS_RPC_PORT: default + `instance` * 2 - 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Amended.

@weiihann
Copy link
Contributor Author

I can see how this flag would be useful when running multiple local nodes on the same host. Because the main use-case for this is for testing, I would opt to not include this flag in Geth.

Primarily, I think users would need to run multiple nodes in this scenario:

  1. Running different nodes on mainnet and testnets
  2. Running different L1 nodes
  3. Running L1 and L2 nodes (perhaps fork of geth)
  4. Testing

For me, I encountered the need to run multiple nodes when I was running 1 mainnet node and 2 testnet nodes. So having this flag would save me some time figuring out the correct ports. For users that are not that familiar with ports, this will save them some trouble.

cmd/utils, node: const for listen and disc ports

amend usage description
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.

None yet

2 participants