-
Notifications
You must be signed in to change notification settings - Fork 529
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
#764 Add GPU support via --enable-features #810
base: master
Are you sure you want to change the base?
Conversation
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.
I'd like to see a little more in the unit tests for this, but aside from that, it looks good 👍
@@ -36,7 +36,7 @@ class MesosTaskBuilderSpec extends SpecificationWithJUnit with Mockito { | |||
|
|||
val parameters = scala.collection.mutable.ListBuffer[Parameter]() | |||
|
|||
val container = Container("dockerImage", ContainerType.DOCKER, volumes, parameters, NetworkMode.HOST, None, networks, forcePullImage = true) |
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.
Please add a new test instead of changing this one.
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.
I'll change the type back to DOCKER.
Do you want some tests for the MesosTaskBuilderSpec, e.g. whether a non-zero value would be applied? And do you want the commits squashed?
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.
@reneploetz According to http://mesos.apache.org/documentation/latest/gpu-support/, seems DOCKER type is not the right one
At the time of this writing, Nvidia GPU support is only available for tasks launched through the Mesos containerizer (i.e. no support exists for launching GPU capable tasks through the Docker containerizer)
Signed-off-by: Rene Ploetz <[email protected]>
@reneploetz let me clarify: I was suggesting you add an additional (new) test with the GPUs enabled, and leave the existing test unchanged. |
What's the status of this PR? Right now, mesos-agents that have GPUs won't be offered to Chronos, which is problematic when all the agents have GPUs. |
I'm afraid I never had time to create a pull request with proper tests as per review - feel free to do so. We were using Chronos with this patch in some product, but switched to another one a while ago. |
This patch adds GPU scheduling support to Chronos the same way Marathon does it:
The argument parsing code was adapted from Marathon (Features.scala / SchedulerConfiguration.scala) to provide the same behaviour.
This should fix #764
Signed-off-by: Rene Ploetz [email protected]