ACCUMULO-4331: first draft #111
Conversation
| PORT("port", Predicates.or(new Bounds(1024, 65535), in(true, "0")), | ||
| "An positive integer in the range 1024-65535, not already in use or specified elsewhere in the configuration"), | ||
| @SuppressWarnings("unchecked") | ||
| PORT("port", Predicates.or(new Bounds(1024, 65535), in(true, "0"), new Matches("\\d{1,5}-\\d{1,5}")), |
There was a problem hiding this comment.
The regex could be a bit more restrictive, like \\d{4,5}-\\d{4,5}
Would there be a benefit for creating a Predicate to verify the port range, like maybe fail sooner (when setting a prop in shell). Instead of validating in getPorts().
|
There are no tests for TServerUtils.startServer. I think I can create some, the problem is that I don't know what ephemeral ports are open on any given test machine. Ideas? |
|
Maybe integrate the build-helper-maven-plugin to reserve ephemeral ports? http://www.mojohaus.org/build-helper-maven-plugin/reserve-network-port-mojo.html |
|
I'll take a look, thanks @ShawnWalker |
|
Added tests to TServerUtils. For some reason the MiniAccumuloCluster tests are failing for me now. Not sure how it is related yet. |
| @@ -168,8 +168,8 @@ public static void init(VolumeManager fs, ServerConfigurationFactory serverConfi | |||
| logConfigWatcher.start(); | |||
|
|
|||
| // Makes sure the log-forwarding to the monitor is configured | |||
| int logPort = conf.getPort(Property.MONITOR_LOG4J_PORT); | |||
| System.setProperty("org.apache.accumulo.core.host.log.port", Integer.toString(logPort)); | |||
| int logPort[] = conf.getPort(Property.MONITOR_LOG4J_PORT); | |||
There was a problem hiding this comment.
Can you put brackets after the type instead of the variable? This is form is valid but discouraged. This occurs a couple different times in the PR.
There was a problem hiding this comment.
Doh! Good catch, didn't intend to do that
|
Tested locally with two tservers and port range appears to be working. |
|
FWIW, with this latest change I was able to start Accumulo to include two tablet servers with setting all of the *.port.client properties to the range of 11000-11006. |
|
Will apply manually. |
| client.shutdownTabletServer(Tracer.traceInfo(), context.rpcCreds(), finalServer, force); | ||
| } | ||
| }); | ||
| for (int port : context.getConfiguration().getPort(Property.TSERV_CLIENTPORT)) { |
There was a problem hiding this comment.
Just realized that this logic is broken for a port of '0'
There was a problem hiding this comment.
Good catch, I wonder how long that has been around.
There was a problem hiding this comment.
Filed https://issues.apache.org/jira/browse/ACCUMULO-4342 for this
|
Being lazy, I suspect the following is the behavior but not 100% sure. @dlmarion hoping you might know off the top of your head since you have been working this. If not I can research it more.
If this is the behavior, then it seems inconsistent to me. NOTE: I edited this comment and changed 7000 to 70,000, which was what I intended to type. |
* Closes issue apache#111 in accumulo-testing
* Closes issue apache#111 in accumulo-testing
* Closes issue #111 in accumulo-testing
First draft. Did not implement the range syntax that was proposed in later comments, but would be easy to swap out. There are some ports where it seemed not to make sense to support a range, log4j monitor port being one as its also set as a system property.