-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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
[SPARK-27835][Core] Resource Scheduling: change driver config from addresses #24730
[SPARK-27835][Core] Resource Scheduling: change driver config from addresses #24730
Conversation
Test build #105871 has finished for PR 24730 at commit
|
Test build #105872 has finished for PR 24730 at commit
|
} finally { | ||
resourceInput.close() | ||
} | ||
resources.map(r => (r.name, new ResourceInformation(r.name, r.addresses))).toMap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could Seq[JsonResourceInformation]
contain duplicated name? might be (very marginally) better to do
resource.toMap.map(...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'm missing what you are saying here with the toMap.map? I can't do the toMap until I do the first map to (name, ResourceInformation), otherwise you just have a Seq[JsonResourceInformation] and toMap doesn't know how to make that a map. If there are 2 resource with the same name when the current code runs the toMap will choose the last one.
I had actually tested this and found the json4s parse and extract actually are handling duplicates as well, looks like it chooses the last one, I couldn't find docs on that behavior though either. The resourcesfile is built by the standalone master/worker so it shouldn't really have duplicates. I'm happy to update though to be explicit so just let me know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yes ;) I just mean it walking through a Seq
when the goal is a Map
LGTM |
thanks Felix, merging to master |
What changes were proposed in this pull request?
Change the Driver resource discovery argument for standalone mode to be a file rather then separate address configs per resource. This makes it consistent with how the Executor is doing it and makes it more flexible in the future, and it makes for less configs if you have multiple resources.
How was this patch tested?
Unit tests and basic manually testing to make sure files were parsed properly.