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

[jvm-packages] Allow specifying host ip from TrackerProperties #3833

Merged
merged 1 commit into from
Oct 27, 2018

Conversation

tovbinm
Copy link
Contributor

@tovbinm tovbinm commented Oct 26, 2018

Allow specifying host ip from the xgboost-tracker.properties file in RabitTracker for Scala

@tovbinm
Copy link
Contributor Author

tovbinm commented Oct 27, 2018

@CodingCat @hcho3 please have a look. Thank you!

@@ -93,8 +93,11 @@ private[scala] class RabitTracker(numWorkers: Int, port: Option[Int] = None,
* @return Boolean flag indicating if the Rabit tracker starts successfully.
*/
private def start(timeout: Duration): Boolean = {
val hostAddress = Option(TrackerProperties.getInstance().getHostIp)
.map(InetAddress.getByName).getOrElse(InetAddress.getLocalHost)
Copy link
Member

Choose a reason for hiding this comment

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

we do not include property file in the released jar, you mean build by your own?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TrackerProperties already handles the case if the property file is missing in the jar. In that case it would return null when calling getHostIp hence the Option.

I borrowed this idea from the java RabitTracker implementation here.

Copy link
Member

Choose a reason for hiding this comment

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

I know, I mean how you add a property file , you have to build by your own

Copy link
Member

Choose a reason for hiding this comment

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

I didn't try adding in application's resource folder

Copy link
Contributor Author

@tovbinm tovbinm Oct 27, 2018

Choose a reason for hiding this comment

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

Correct. In my project I have to add resources/xgboost-tracker.properties file. For example with the following contents:

host-ip=0.0.0.0

Copy link
Member

Choose a reason for hiding this comment

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

Good!

@CodingCat
Copy link
Member

Thanks for the contribution!, merged

@CodingCat CodingCat merged commit d81fedb into dmlc:master Oct 27, 2018
@tovbinm
Copy link
Contributor Author

tovbinm commented Oct 27, 2018

@CodingCat thanks. What's the ETA on the 0.81 release?

@tovbinm tovbinm deleted the mt/tracker-host-port branch October 27, 2018 05:02
@CodingCat
Copy link
Member

@tovbinm it's currently blocking at some multi-threading perf issue, I am not sure who is actively looking into that @hcho3 ?

@hcho3
Copy link
Collaborator

hcho3 commented Oct 27, 2018

I've been working on it. November 1 is the day. I may decide to ignore the issue if it takes too much time.

alois-bissuel pushed a commit to criteo-forks/xgboost that referenced this pull request Dec 4, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jan 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants