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

HIVE-24426. Spark job fails with fixed LlapTaskUmbilicalServer port. #1705

Merged
merged 5 commits into from
Dec 1, 2020

Conversation

ayushtkn
Copy link
Member

portFound = true;
} else {
int maxPort = Integer.parseInt(portRange[1]);
for (int i = minPort; i < maxPort; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

portFound = true;
break;
} catch (BindException be) {
// Ignore and move ahead, in search of a free port.
Copy link
Contributor

Choose a reason for hiding this comment

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

Log at warn level to say which port is being tried and what error message received for debugging.

.setNumHandlers(numHandlers).setSecretManager(jobTokenSecretManager)
.build();
if (conf
.getBoolean(CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHORIZATION,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: you could move this to a private variable instead of searching conf object every time (now that startServer is in a loop)

"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"),
Copy link
Contributor

Choose a reason for hiding this comment

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

add what the min and max values for the range can be (1024 - 65535)

portFound = true;
break;
} catch (BindException be) {
// Ignore and move ahead, in search of a free port.
Copy link
Contributor

Choose a reason for hiding this comment

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

same here for logging

startServerInternal(conf, minPort, numHandlers, jobTokenSecretManager);
portFound = true;
} else {
int maxPort = Integer.parseInt(portRange[1]);
Copy link
Contributor

Choose a reason for hiding this comment

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

same here (use RangeValidator)

.setNumHandlers(numHandlers).setSecretManager(jobTokenSecretManager)
.build();

if (conf
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: same here to move this to private variable.

@ayushtkn
Copy link
Member Author

Thanx @prasanthj for the review, I have handled the review comments. Please have a look

Copy link
Contributor

@prasanthj prasanthj left a comment

Choose a reason for hiding this comment

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

lgtm.
nit: will be good if you can add a success log message with the port number that was successfully bound.

@ayushtkn
Copy link
Member Author

ayushtkn commented Dec 1, 2020

Thanx @prasanthj for the review, I have added the success log as well. Please have a check.

@prasanthj prasanthj merged commit c636bdf into apache:master Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants