Add unified port Property #5966

DomGarguilo · 2025-10-30T19:49:08Z

All services now read a new rpc.bind.port Property for their port candidate list. This new prop has a default value of 19000-19999. Individual port props have been removed (*_CLIENTPORT properties)
When only one port is set for this property, the internal value will have the next 1000 ports appended to allow for the next 1000 ports to be searched for the next open port after the given port
The port range syntax is just an inclusive range like M-N

All the changed tests and ITs pass and so does the sunny maven profile.

dlmarion · 2025-11-17T21:29:29Z

Is there an example for how a user would configure ports for the individual servers? There is a comment in #5845 about setting this in accumulo-env.sh, but I'm not sure that will work. I'm thinking about the case where there are multiple tservers or compactors running on the same host. With the current configuration you can make sure that tservers are allocating ports from set A and compactors from set B. I think with this new configuration, all processes on the host will be allocating ports from a single set of available ports.

DomGarguilo · 2025-11-19T19:51:32Z

Is there an example for how a user would configure ports for the individual servers? There is a comment in #5845 about setting this in accumulo-env.sh, but I'm not sure that will work. I'm thinking about the case where there are multiple tservers or compactors running on the same host. With the current configuration you can make sure that tservers are allocating ports from set A and compactors from set B. I think with this new configuration, all processes on the host will be allocating ports from a single set of available ports.

Yea I think you're right. The port range prop can of course be set when manually starting a component but I don't think it can be added to accumulo-env.sh without significant changes at least.

One way that seems like it could work well is to add a port range field to cluster.yaml. That way we could specify the port range per component type.

dlmarion · 2025-11-19T20:10:16Z

One way that seems like it could work well is to add a port range field to cluster.yaml. That way we could specify the port range per component type.

I guess another way would be to create a new variable that can be added to the end of

accumulo/assemble/bin/accumulo

Line 89 in 8177db1

exec "${JAVA[@]}" "${JAVA_OPTS[@]}" org.apache.accumulo.start.Main "$@"

This would allow that variable to be set in accumulo-env.sh

DomGarguilo · 2025-11-19T20:23:19Z

One way that seems like it could work well is to add a port range field to cluster.yaml. That way we could specify the port range per component type.

I guess another way would be to create a new variable that can be added to the end of

accumulo/assemble/bin/accumulo

Line 89 in 8177db1

exec "${JAVA[@]}" "${JAVA_OPTS[@]}" org.apache.accumulo.start.Main "$@"

This would allow that variable to be set in accumulo-env.sh

Yea that would work too. Thinking through adding it to cluster.yaml, it might look something like this:

  tserver:
    default:
      servers_per_host: 2
      hosts:
        - localhost1
        - localhost2
      port_range: 20000-20049
    highmem:
      servers_per_host: 1
      hosts:
        - hmvm1
      port_range: 20050-20099

So we would define a range per component type per resource group. Not sure that we want/need that kind of fine grained control or if we just want ranges per component type regardless of RG.

dlmarion · 2025-11-19T20:28:54Z

I think the port range needs to be able to be specified at the server type level. The Accumulo client needs to be able to interact with the Manager, ScanServer, and TabletServer processes. The client doesn't interact with the Compactor, GarbageCollector, or Monitor. There is no good way to configure a firewall to only allow clients to talk with the Manager, ScanServer, and TabletServer with a single port range being used for all processes.

Edit: I re-read your example, and that might work.

ddanielr · 2025-11-19T21:54:19Z

Moving the range property to the service definition inside cluster.yaml is a solid improvement as I've been bitten before by forgetting to also increase port ranges after raising the compactors_per_host value in cluster.yaml.

If adding this property to cluster.yaml isn't desired then we might be able to use the include option to have service specific property files IF property file contents collapse with a "last one wins" override behavior.

If each file has to contain distinct property definitions then the existing accumulo.properties file would need to be separated into multiple files like general.properties, tserver.properties,manager.properties, etc.

dlmarion · 2025-11-19T22:07:45Z

I think the port range property that is sent down to accumulo should be added to the list of arguments to the java command (-o rpc.bind.port=N-M). See #5966 (comment) for context.

For example, if accumulo used $EXTRA_ARGS at the end of that line, then that could be set in accumulo-env.sh for each process type per resource group here. The modifications that you have above to accumulo-cluster could just set the $EXTRA_ARGS variable when calling accumulo-service.

keith-turner · 2025-11-19T22:51:09Z

So we would define a range per component type per resource group. Not sure that we want/need that kind of fine grained control or if we just want ranges per component type regardless of RG.

As mentioned, may be able to do this already in accumulo-env.sh using -o. Also, can this be accomplished by setting the prop at a RG level in zookeeper?

ddanielr · 2025-11-19T23:38:06Z

So we would define a range per component type per resource group. Not sure that we want/need that kind of fine grained control or if we just want ranges per component type regardless of RG.

As mentioned, may be able to do this already in accumulo-env.sh using -o. Also, can this be accomplished by setting the prop at a RG level in zookeeper?

We currently can't do this in accumulo-env.sh because all of the JAVA_OPTS are set before calling start.Main.
So "-o" is interpreted by java as an invalid opt

accumulo/assemble/bin/accumulo

Line 89 in b29f014

exec "${JAVA[@]}" "${JAVA_OPTS[@]}" org.apache.accumulo.start.Main "$@"

We used to support EXTRA_ARGS by accumulo-cluster

accumulo/assemble/bin/accumulo-cluster

Lines 166 to 167 in 8f52dd1

    
           EXTRA_ARGS="${*:4}" 
        
           $SSH "$host" "bash -c 'ACCUMULO_SERVICE_INSTANCE=${ACCUMULO_SERVICE_INSTANCE} ${bin}/accumulo-service \"$service\" \"$control_cmd\" \"-a\" \"$host\" $EXTRA_ARGS '"

But that was removed in 2.1.4

./bin/accumulo-cluster start --tservers -o tserver.port.client=9954
accumulo-cluster: invalid option -- 'o'
Usage: accumulo-cluster <command> [<option> ...]

@dlmarion is right, we would need to create another var that gets passed to accumulo-service and then added to the end of the Main bit in the accumulo wrapper script.

DomGarguilo · 2025-11-24T19:58:23Z

In aa5b11a I made it so we can set per-server-component-type properties in accumulo-env.sh. I left a commented example in the file but there may be a better example to use that the one I included.

I tested with the following in accumulo-env.sh:

case "$cmd" in
  monitor) ACCUMULO_MAIN_ARGS=(-o "rpc.bind.port=9995") ;;
  tserver)   ACCUMULO_MAIN_ARGS=(-o "rpc.bind.port=20000-20049") ;;
  compactor) ACCUMULO_MAIN_ARGS=(-o "rpc.bind.port=20050-20099") ;;
  sserver) ACCUMULO_MAIN_ARGS=(-o "rpc.bind.port=20100-20149") ;;
esac

and it all seemed to work as expected:

dgarguilo@thor: ~/github/fluo-uno main!
$ accumulo-service tserver list                                                                                                                                                                 [14:20:48]
Currently running tserver processes (fields: process pid port):
tserver_default_1 55482 20003
tserver_default_2 55485 20000
tserver_default_3 55488 20001
tserver_default_4 55491 20002
 
dgarguilo@thor: ~/github/fluo-uno main!
$ accumulo-service compactor list                                                                                                                                                               [14:20:53]
Currently running compactor processes (fields: process pid port):
compactor_default_1 55550 20051
compactor_default_2 55556 20050
compactor_default_3 55559 20052
 
dgarguilo@thor: ~/github/fluo-uno main!
$ accumulo-service sserver list                                                                                                                                                                 [14:21:10]
Currently running sserver processes (fields: process pid port):
sserver_default_1 55534 20101
sserver_default_2 55539 20100
 
dgarguilo@thor: ~/github/fluo-uno main!
$ accumulo-service monitor list                                                                                                                                                                 [14:21:29]
Currently running monitor processes (fields: process pid port):
monitor_default_1 55508 9995

assemble/conf/accumulo-env.sh

dlmarion · 2025-11-24T20:37:44Z

assemble/bin/accumulo

+  # Allow users to supply extra Accumulo arguments via ACCUMULO_MAIN_ARGS
+  if ! declare -p ACCUMULO_MAIN_ARGS &>/dev/null; then
+    ACCUMULO_MAIN_ARGS=()
+  else
+    declare_output=$(declare -p ACCUMULO_MAIN_ARGS 2>/dev/null)
+    if [[ $declare_output != declare\ -a* ]]; then
+      ACCUMULO_MAIN_ARGS_RAW=${ACCUMULO_MAIN_ARGS[*]}
+      read -r -a ACCUMULO_MAIN_ARGS <<<"${ACCUMULO_MAIN_ARGS_RAW}"
+    fi
+  fi


Curious why this is handled differently than how JAVA_OPTS is handled, is there a reason?

So, I have a couple of thoughts on this:

This is making me really regret that I didn't push harder against the -o property passing. My original proposal when we reworked the launch scripts was to support Java system properties for accumulo configs, as in: -Daccumulo.rpc.bind.port=9999. I don't know the reason Mike implemented the -o, but it is really making it inconvenient here. If we could just pass them as system properties, then this would be a non-issue. And, I still think that's the better implementation.

The default value should be a range that servers of any type can share. So, most of the time, users don't need to specify anything in their config files.

If one is going to specify properties in config files, we don't need any of this, because we can inject properties into the commons-configuration file using property interpolation. If a user wants to set a custom port, they can either use a shared accumulo.properties file that has an entry like rpc.bind.port=${env.RPC_PORT} and then do export RPC_PORT=9999 in accumulo-env.sh based on the server type, or they can use something like rpc.bind.port=${sys:accumulo.rpc.bind.port} and set -Daccumulo.rpc.bind.port=9999 in their JAVA_OPTS based on the server type, OR they can import server-type-specific properties using includes that pull in a specific config for a specific server type, and they can spread their config over multiple config files, OR they can choose different wholly independent config files, by adding -Daccumulo.properties=/path/to/server-specific.properties in their accumulo-env.sh based on the server type.

So, we just don't need this. We already have better options than making the scripts more complicated.

Regarding point # 3, it would be useful to have an example of how the includes would work. If you understand commons-configuration and how it works, then it's obvious. If you don't, then it's not obvious and the user is unlikely to even think about it as a viable option.

core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java

dlmarion · 2025-11-24T20:47:58Z

core/src/main/java/org/apache/accumulo/core/conf/Property.java

+  RPC_BIND_PORT("rpc.bind.port", "19000-19999", PropertyType.PORT,
+      """
+          The port or range of ports servers attempt to bind for RPC traffic. Provide a single \
+          value to target an exact port (will attempt higher ports if given port is already in use, \
+          up to 1000 additional checks), or a range using formats like '19000-19999' to allow searching for \
+          the first available port within that range.
+          """,
+      "4.0.0"),


will attempt higher ports if given port is already in use, up to 1000 additional checks

I don't think we should do this. If the user specifies a single port, then we should honor it. If they want to use port search, then they should specify a range.

I agree. This was one of the things in the original ticket:

A single integer value, A, should be interpreted as the range [A, A+1), and implies no port searching.... use that one value or fail.

Fixed in d6d8553. Only ports in the given range will be used.

ctubbsii · 2025-11-25T01:01:18Z

assemble/bin/accumulo

+  # Allow users to supply extra Accumulo arguments via ACCUMULO_MAIN_ARGS
+  if ! declare -p ACCUMULO_MAIN_ARGS &>/dev/null; then
+    ACCUMULO_MAIN_ARGS=()
+  else
+    declare_output=$(declare -p ACCUMULO_MAIN_ARGS 2>/dev/null)
+    if [[ $declare_output != declare\ -a* ]]; then
+      ACCUMULO_MAIN_ARGS_RAW=${ACCUMULO_MAIN_ARGS[*]}
+      read -r -a ACCUMULO_MAIN_ARGS <<<"${ACCUMULO_MAIN_ARGS_RAW}"
+    fi
+  fi


So, I have a couple of thoughts on this:

This is making me really regret that I didn't push harder against the -o property passing. My original proposal when we reworked the launch scripts was to support Java system properties for accumulo configs, as in: -Daccumulo.rpc.bind.port=9999. I don't know the reason Mike implemented the -o, but it is really making it inconvenient here. If we could just pass them as system properties, then this would be a non-issue. And, I still think that's the better implementation.

The default value should be a range that servers of any type can share. So, most of the time, users don't need to specify anything in their config files.

If one is going to specify properties in config files, we don't need any of this, because we can inject properties into the commons-configuration file using property interpolation. If a user wants to set a custom port, they can either use a shared accumulo.properties file that has an entry like rpc.bind.port=${env.RPC_PORT} and then do export RPC_PORT=9999 in accumulo-env.sh based on the server type, or they can use something like rpc.bind.port=${sys:accumulo.rpc.bind.port} and set -Daccumulo.rpc.bind.port=9999 in their JAVA_OPTS based on the server type, OR they can import server-type-specific properties using includes that pull in a specific config for a specific server type, and they can spread their config over multiple config files, OR they can choose different wholly independent config files, by adding -Daccumulo.properties=/path/to/server-specific.properties in their accumulo-env.sh based on the server type.

So, we just don't need this. We already have better options than making the scripts more complicated.

assemble/conf/accumulo-env.sh

ctubbsii · 2025-11-25T01:06:33Z

core/src/main/java/org/apache/accumulo/core/conf/Property.java

-  MANAGER_CLIENTPORT("manager.port.client", "9999", PropertyType.PORT,
-      "The port used for handling client connections on the manager.", "1.3.5"),


It's probably a good idea to keep these around and deprecated. If the new property is set, then we should use the new behavior and ignore anything in the old properties. Otherwise, we should read the old properties and use them.

Ideally, we'd introduce this in 3.1 and deprecate them there, so we don't need to have these in 4.0 at all.

Because this is a much simpler config, that hopefully users just don't have to worry about much because the defaults are reasonable, I don't think it would be completely terrible if we just changed this without a deprecation, but we would definitely need to call that out in the release notes.

ctubbsii · 2025-11-25T01:20:11Z

core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java

+      if (input == null) {
+        return true;
+      }
+      final String trimmed = input.trim();


If we're trimming for validation, we need to trim when we use it. It might make sense here, but I think it would be novel. Looking at the code, we only trim the input in PropertyType in two other places, and one might be a bug.

We trim when we strip the units off of the end of bounded units, so we can isolate the numeric part, because we're only testing the number part.

We trim absolute paths during validation (might be a bug... seems sketchy at a glance)

In general, we assume that the property value is exactly what the user specified, including any spaces. I don't know that we should always do that, but it would be nice if we were consistent, and it doesn't look like we do anything to transform the user's input by stripping off whitespace in most of the other property types, so we probably shouldn't here.

Removed the use of .trim in 7375e2d. Added a return false to PortPredicate() if the value contains any whitespace in that commit too. That will cause PropertyType.PORT.isValidFormat() to return false before the whitespace causes issues further down the line.

ctubbsii · 2025-11-25T01:21:01Z

core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java

+      if ("0".equals(trimmed)) {
+        return true;
+      }


Why is 0 valid here? It seems like it shouldn't be.

There are several existing places in the code where we set "0" as the value for the port:

accumulo/minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloConfigImpl.java

Lines 292 to 296 in 4c259d7

private void mergePropWithRandomPort(String key) {

if (!siteConfig.containsKey(key)) {

siteConfig.put(key, "0");

}

}

This is used for all the

accumulo/minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloConfigImpl.java

Lines 186 to 189 in 4c259d7

mergePropWithRandomPort(Property.MANAGER_CLIENTPORT.getKey());

mergePropWithRandomPort(Property.TSERV_CLIENTPORT.getKey());

mergePropWithRandomPort(Property.MONITOR_PORT.getKey());

mergePropWithRandomPort(Property.GC_PORT.getKey());

It looks like eventually the prop gets used in the server builder code to create a new InetSocketAddress(host, port) whose javadoc says "A valid port value is between 0 and 65535. A port number of zero will let the system pick up an ephemeral port in a bind operation."

It would be a change in functionality to disallow "0".

There are several other places as well as tests and ITs that set 0 for the port. Maybe we could document this better.

Added to the docs for RPC_BIND_PORT in d6d8553

DomGarguilo · 2025-11-26T17:50:37Z

Here are some notes on what I have tried:

env interpolation

In accumulo.properties set rpc.bind.port=${env:RPC_PORT:-19000-19999}
In accumulo-env.sh set:

case "$cmd" in
  sserver)   export RPC_PORT="20000-20049" ;;
  compactor) export RPC_PORT="20050-20099" ;;
  tserver)   export RPC_PORT="20100-20149" ;;
  monitor)   export RPC_PORT="9995" ;;
esac

One thing to note is the RPC_PORT env var needs to be set beforehand or else we get ERROR: FATAL: BAD CONFIG improperly formatted value for key (rpc.bind.port, type=port) : ${env:RPC_PORT} for site config. Thats why we have to do rpc.bind.port=${env:RPC_PORT:-19000-19999} and not rpc.bind.port=${env:RPC_PORT}. That way the env var will just be replaced if present, otherwise it defaults to 19000-19999.

sysprop

Similar to the env interpolation. Set rpc.bind.port=${sys:accumulo.rpc.bind.port:-19000-19999} in accumulo.properties
and in accumulo-env.sh:

case "$cmd" in
  tserver)   JAVA_OPTS=("-Daccumulo.rpc.bind.port=20001-20049" "${JAVA_OPTS[@]}") ;;
  compactor) JAVA_OPTS=("-Daccumulo.rpc.bind.port=20051-20099" "${JAVA_OPTS[@]}") ;;
  sserver)   JAVA_OPTS=("-Daccumulo.rpc.bind.port=20101-20149" "${JAVA_OPTS[@]}") ;;
  monitor)   JAVA_OPTS=("-Daccumulo.rpc.bind.port=9995" "${JAVA_OPTS[@]}") ;;
esac

commons configuration `includes`

I am still trying to get this working but no luck yet

Add unified port Property

3cf73c0

DomGarguilo added this to the 4.0.0 milestone Oct 30, 2025

DomGarguilo self-assigned this Oct 30, 2025

DomGarguilo requested a review from ctubbsii October 30, 2025 19:49

Merge remote-tracking branch 'upstream/main' into bindPort

d4ddee3

DomGarguilo added 2 commits November 24, 2025 12:30

Merge remote-tracking branch 'upstream/main' into bindPort

c364fed

Add ability to set accumulo props in accumulo-env.sh

aa5b11a

properly read array

120dce0

dlmarion reviewed Nov 24, 2025

View reviewed changes

ctubbsii requested changes Nov 25, 2025

View reviewed changes

DomGarguilo added 3 commits November 25, 2025 11:24

rollback changes to scripts

16da39a

remove .trim() use isValidFormat() in getPortStream()

7375e2d

add to property docs, improve tests

d6d8553

DomGarguilo added 2 commits November 28, 2025 09:28

Merge remote-tracking branch 'upstream/main' into bindPort

8e2719f

revert to simpler code in TServerUtils

81d398f

		MANAGER_CLIENTPORT("manager.port.client", "9999", PropertyType.PORT,
		"The port used for handling client connections on the manager.", "1.3.5"),

	private void mergePropWithRandomPort(String key) {
	if (!siteConfig.containsKey(key)) {
	siteConfig.put(key, "0");
	}
	}

	mergePropWithRandomPort(Property.MANAGER_CLIENTPORT.getKey());
	mergePropWithRandomPort(Property.TSERV_CLIENTPORT.getKey());
	mergePropWithRandomPort(Property.MONITOR_PORT.getKey());
	mergePropWithRandomPort(Property.GC_PORT.getKey());

Add unified port Property #5966

Are you sure you want to change the base?

Add unified port Property #5966

Conversation

DomGarguilo commented Oct 30, 2025

Uh oh!

dlmarion commented Nov 17, 2025

Uh oh!

DomGarguilo commented Nov 19, 2025

Uh oh!

dlmarion commented Nov 19, 2025

Uh oh!

DomGarguilo commented Nov 19, 2025

Uh oh!

dlmarion commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ddanielr commented Nov 19, 2025

Uh oh!

dlmarion commented Nov 19, 2025

Uh oh!

keith-turner commented Nov 19, 2025

Uh oh!

ddanielr commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DomGarguilo commented Nov 24, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DomGarguilo Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DomGarguilo commented Nov 26, 2025

env interpolation

sysprop

commons configuration includes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dlmarion commented Nov 19, 2025 •

edited

Loading

ddanielr commented Nov 19, 2025 •

edited

Loading

DomGarguilo Nov 25, 2025 •

edited

Loading

commons configuration `includes`