prasanthj commented on a change in pull request #1705:
URL: https://github.com/apache/hive/pull/1705#discussion_r532410351
##########
File path:
llap-client/src/java/org/apache/hadoop/hive/llap/tezplugins/helpers/LlapTaskUmbilicalServer.java
##########
@@ -54,27 +56,54 @@
public LlapTaskUmbilicalServer(Configuration conf, LlapTaskUmbilicalProtocol
umbilical, int numHandlers) throws IOException {
jobTokenSecretManager = new JobTokenSecretManager();
- int umbilicalPort = HiveConf.getIntVar(conf,
HiveConf.ConfVars.LLAP_TASK_UMBILICAL_SERVER_PORT);
- if (umbilicalPort <= 0) {
- umbilicalPort = 0;
+
+ String[] portRange =
+ conf.get(HiveConf.ConfVars.LLAP_TASK_UMBILICAL_SERVER_PORT.varname)
+ .split("-");
+
+ int minPort = Integer.parseInt(portRange[0]);
+ boolean portFound = false;
+ IOException e = null;
+ if (portRange.length == 1) {
+ // Single port specified, not Range.
+ startServer(conf, umbilical, numHandlers, minPort);
+ portFound = true;
+ } else {
+ int maxPort = Integer.parseInt(portRange[1]);
+ for (int i = minPort; i < maxPort; i++) {
+ try {
+ startServer(conf, umbilical, numHandlers, i);
+ portFound = true;
+ break;
+ } catch (BindException be) {
+ // Ignore and move ahead, in search of a free port.
Review comment:
Log at warn level to say which port is being tried and what error
message received for debugging.
##########
File path:
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java
##########
@@ -257,23 +259,32 @@ protected void startRpcServer() {
int numHandlers =
HiveConf.getIntVar(conf,
ConfVars.LLAP_TASK_COMMUNICATOR_LISTENER_THREAD_COUNT);
- int umbilicalPort = HiveConf.getIntVar(conf,
ConfVars.LLAP_TASK_UMBILICAL_SERVER_PORT);
- if (umbilicalPort <= 0) {
- umbilicalPort = 0;
+ String[] portRange =
+ conf.get(HiveConf.ConfVars.LLAP_TASK_UMBILICAL_SERVER_PORT.varname)
+ .split("-");
+ boolean portFound = false;
+ IOException ioe = null;
+ int minPort = Integer.parseInt(portRange[0]);
+ if (portRange.length == 1) {
+ // Single port specified, not range.
+ startServerInternal(conf, minPort, numHandlers, jobTokenSecretManager);
+ portFound = true;
+ } else {
+ int maxPort = Integer.parseInt(portRange[1]);
+ for (int i = minPort; i < maxPort; i++) {
+ try {
+ startServerInternal(conf, i, numHandlers, jobTokenSecretManager);
+ portFound = true;
+ break;
+ } catch (BindException be) {
+ // Ignore and move ahead, in search of a free port.
Review comment:
same here for logging
##########
File path:
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java
##########
@@ -283,6 +294,23 @@ protected void startRpcServer() {
}
}
+ private void startServerInternal(Configuration conf, int umbilicalPort,
+ int numHandlers, JobTokenSecretManager jobTokenSecretManager)
+ throws IOException {
+ server = new RPC.Builder(conf).setProtocol(LlapTaskUmbilicalProtocol.class)
+
.setBindAddress("0.0.0.0").setPort(umbilicalPort).setInstance(umbilical)
+ .setNumHandlers(numHandlers).setSecretManager(jobTokenSecretManager)
+ .build();
+
+ if (conf
Review comment:
nit: same here to move this to private variable.
##########
File path:
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java
##########
@@ -257,23 +259,32 @@ protected void startRpcServer() {
int numHandlers =
HiveConf.getIntVar(conf,
ConfVars.LLAP_TASK_COMMUNICATOR_LISTENER_THREAD_COUNT);
- int umbilicalPort = HiveConf.getIntVar(conf,
ConfVars.LLAP_TASK_UMBILICAL_SERVER_PORT);
- if (umbilicalPort <= 0) {
- umbilicalPort = 0;
+ String[] portRange =
+ conf.get(HiveConf.ConfVars.LLAP_TASK_UMBILICAL_SERVER_PORT.varname)
+ .split("-");
+ boolean portFound = false;
+ IOException ioe = null;
+ int minPort = Integer.parseInt(portRange[0]);
+ if (portRange.length == 1) {
+ // Single port specified, not range.
+ startServerInternal(conf, minPort, numHandlers, jobTokenSecretManager);
+ portFound = true;
+ } else {
+ int maxPort = Integer.parseInt(portRange[1]);
Review comment:
same here (use RangeValidator)
##########
File path:
llap-client/src/java/org/apache/hadoop/hive/llap/tezplugins/helpers/LlapTaskUmbilicalServer.java
##########
@@ -54,27 +56,54 @@
public LlapTaskUmbilicalServer(Configuration conf, LlapTaskUmbilicalProtocol
umbilical, int numHandlers) throws IOException {
jobTokenSecretManager = new JobTokenSecretManager();
- int umbilicalPort = HiveConf.getIntVar(conf,
HiveConf.ConfVars.LLAP_TASK_UMBILICAL_SERVER_PORT);
- if (umbilicalPort <= 0) {
- umbilicalPort = 0;
+
+ String[] portRange =
+ conf.get(HiveConf.ConfVars.LLAP_TASK_UMBILICAL_SERVER_PORT.varname)
+ .split("-");
+
+ int minPort = Integer.parseInt(portRange[0]);
+ boolean portFound = false;
+ IOException e = null;
+ if (portRange.length == 1) {
+ // Single port specified, not Range.
+ startServer(conf, umbilical, numHandlers, minPort);
+ portFound = true;
+ } else {
+ int maxPort = Integer.parseInt(portRange[1]);
+ for (int i = minPort; i < maxPort; i++) {
Review comment:
can you use RangeValidator from HiveConf and make sure the min is 1024
and max is 65535 which is the port range for user space ports.
##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -4917,8 +4917,9 @@ private static void populateLlapDaemonVarsSet(Set<String>
llapDaemonVarsSetLocal
"Sleep duration (in milliseconds) to wait before retrying on error when
obtaining a\n" +
"connection to LLAP daemon from Tez AM.",
"llap.task.communicator.connection.sleep-between-retries-millis"),
- LLAP_TASK_UMBILICAL_SERVER_PORT("hive.llap.daemon.umbilical.port", 0,
- "LLAP task umbilical server RPC port"),
+ LLAP_TASK_UMBILICAL_SERVER_PORT("hive.llap.daemon.umbilical.port", "0",
+ "LLAP task umbilical server RPC port or range of ports to try in case "
+ + "the first port is occupied"),
Review comment:
add what the min and max values for the range can be (1024 - 65535)
##########
File path:
llap-client/src/java/org/apache/hadoop/hive/llap/tezplugins/helpers/LlapTaskUmbilicalServer.java
##########
@@ -54,27 +56,54 @@
public LlapTaskUmbilicalServer(Configuration conf, LlapTaskUmbilicalProtocol
umbilical, int numHandlers) throws IOException {
jobTokenSecretManager = new JobTokenSecretManager();
- int umbilicalPort = HiveConf.getIntVar(conf,
HiveConf.ConfVars.LLAP_TASK_UMBILICAL_SERVER_PORT);
- if (umbilicalPort <= 0) {
- umbilicalPort = 0;
+
+ String[] portRange =
+ conf.get(HiveConf.ConfVars.LLAP_TASK_UMBILICAL_SERVER_PORT.varname)
+ .split("-");
+
+ int minPort = Integer.parseInt(portRange[0]);
+ boolean portFound = false;
+ IOException e = null;
+ if (portRange.length == 1) {
+ // Single port specified, not Range.
+ startServer(conf, umbilical, numHandlers, minPort);
+ portFound = true;
+ } else {
+ int maxPort = Integer.parseInt(portRange[1]);
+ for (int i = minPort; i < maxPort; i++) {
+ try {
+ startServer(conf, umbilical, numHandlers, i);
+ portFound = true;
+ break;
+ } catch (BindException be) {
+ // Ignore and move ahead, in search of a free port.
+ e = be;
+ }
+ }
}
- server = new RPC.Builder(conf)
- .setProtocol(LlapTaskUmbilicalProtocol.class)
- .setBindAddress("0.0.0.0")
- .setPort(umbilicalPort)
- .setInstance(umbilical)
- .setNumHandlers(numHandlers)
- .setSecretManager(jobTokenSecretManager).build();
-
- if
(conf.getBoolean(CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHORIZATION,
false)) {
- server.refreshServiceAcl(conf, new
LlapUmbilicalExternalPolicyProvider());
+ if (!portFound) {
+ throw e;
}
- server.start();
this.address = NetUtils.getConnectAddress(server);
LOG.info(
"Started TaskUmbilicalServer: " + umbilical.getClass().getName() + "
at address: " + address +
- " with numHandlers=" + numHandlers);
+ " with numHandlers=" + numHandlers);
+ }
+
+ private void startServer(Configuration conf,
+ LlapTaskUmbilicalProtocol umbilical, int numHandlers, int port)
+ throws IOException {
+ server = new RPC.Builder(conf).setProtocol(LlapTaskUmbilicalProtocol.class)
+ .setBindAddress("0.0.0.0").setPort(port).setInstance(umbilical)
+ .setNumHandlers(numHandlers).setSecretManager(jobTokenSecretManager)
+ .build();
+ if (conf
+
.getBoolean(CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHORIZATION,
Review comment:
nit: you could move this to a private variable instead of searching conf
object every time (now that startServer is in a loop)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
