dlmarion commented on code in PR #5966:
URL: https://github.com/apache/accumulo/pull/5966#discussion_r2557657398
##########
assemble/conf/accumulo-env.sh:
##########
@@ -159,3 +159,13 @@ esac
## environment, that will override what is set here, rather than some mangled
## merged result. You can set the variable any way you like.
#declare -p 'ACCUMULO_JAVA_PREFIX' &>/dev/null || ACCUMULO_JAVA_PREFIX=''
+
Review Comment:
Can we add these to lines 97-102 above?
##########
core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java:
##########
@@ -347,21 +347,27 @@ public IntStream getPortStream(Property property) {
checkType(property, PropertyType.PORT);
String portString = get(property);
- try {
+ Preconditions.checkArgument(portString != null, "Port cannot be null.");
+ portString = portString.trim();
+ Preconditions.checkArgument(!portString.isEmpty(), "Port cannot be
empty.");
Review Comment:
Could this be replaced with `PropertyType.PORT.isValidFormat()` ?
##########
assemble/bin/accumulo:
##########
@@ -86,7 +86,18 @@ function main() {
JAVA=("${ACCUMULO_JAVA_PREFIX[@]}" "$JAVA")
fi
- exec "${JAVA[@]}" "${JAVA_OPTS[@]}" org.apache.accumulo.start.Main "$@"
+ # 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
Review Comment:
Curious why this is handled differently than how JAVA_OPTS is handled, is
there a reason?
##########
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##########
@@ -95,6 +95,14 @@ public enum Property {
The local IP address to which this server should bind for sending \
and receiving network traffic. If not set then the process binds to all
addresses.
""", "2.1.4"),
+ 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"),
Review Comment:
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
