tgravescs commented on a change in pull request #26284: 
[SPARK-29415][Core]Stage Level Sched: Add base ResourceProfile and Request 
classes
URL: https://github.com/apache/spark/pull/26284#discussion_r344332083
 
 

 ##########
 File path: 
core/src/main/scala/org/apache/spark/resource/ExecutorResourceRequest.scala
 ##########
 @@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.resource
+
+/**
+ * An Executor resource request. This is used in conjunction with the 
ResourceProfile to
+ * programmatically specify the resources needed for an RDD that will be 
applied at the
+ * stage level.
+ *
+ * This is used to specify what the resource requirements are for an Executor 
and how
+ * Spark can find out specific details about those resources. Not all the 
parameters are
+ * required for every resource type. The resources names supported
+ * correspond to the regular Spark configs with the prefix removed. For 
instance overhead
+ * memory in this api is memoryOverhead, which is 
spark.executor.memoryOverhead with
+ * spark.executor removed. Resources like GPUs are resource.gpu
+ * (spark configs spark.executor.resource.gpu.*). The amount, discoveryScript, 
and vendor
+ * parameters for resources are all the same parameters a user would specify 
through the
+ * configs: spark.executor.resource.{resourceName}.{amount, discoveryScript, 
vendor}.
+ *
+ * For instance, a user wants to allocate an Executor with GPU resources on 
YARN. The user has
+ * to specify the resource name (resource.gpu), the amount or number of GPUs 
per Executor,
+ * units would not be used as its not a memory config, the discovery script 
would be specified
+ * so that when the Executor starts up it can discovery what GPU addresses are 
available for it to
+ * use because YARN doesn't tell Spark that, then vendor would not be used 
because
+ * its specific for Kubernetes.
+ *
+ * See the configuration and cluster specific docs for more details.
+ *
+ * There are alternative constructors for working with Java.
+ *
+ * @param resourceName Name of the resource
+ * @param amount Amount requesting
+ * @param units Optional units of the amount. For things like Memory, default 
is no units, only byte
+ *              types (b, mb, gb, etc) are currently supported.
 
 Review comment:
   I quickly ran into issues with the second option I talked about above 
because the TaskResourceRequest and ExecutorResourceRequest then become mutable 
which I think complicates things.
   
   I came up with a compromise that I think still gives us ability to add 
things like .prefer to ResourceProfiles and that is just adding an intermediate 
class TAskResourceRequests and ExecutorResourceRequests (note the s on the 
end). In these classes it adds the convenient setters for cpu, memory, etc. But 
it also leaves the base ExecutorResourceRequest and TaskResourceRequest 
classes.  I was going to make those internal classes but the downside to that 
is if we want the user to be able to get the resources back from the 
ExecutorResourceRequests those classes are essentially needed.
   
   Here is an example:
   
   With the existing PR:
   ```
   -    val cpuTaskReq = new TaskResourceRequest(ResourceProfile.CPUS, 1)
   -    val coresExecReq = new ExecutorResourceRequest(ResourceProfile.CORES, 2)
   -    val memExecReq = new ExecutorResourceRequest(ResourceProfile.MEMORY, 
4096, "mb")
   -    val omemExecReq = new 
ExecutorResourceRequest(ResourceProfile.OVERHEAD_MEM, 2048)
   -    val pysparkMemExecReq = new 
ExecutorResourceRequest(ResourceProfile.PYSPARK_MEM, 1024)
   -
   -    rprof.require(cpuTaskReq)
   -    rprof.require(coresExecReq)
   -    rprof.require(memExecReq)
   -    rprof.require(omemExecReq)
   -    rprof.require(pysparkMemExecReq)
   ```
   
   With the changes I talked about adding in ExecutorResourceRequests and 
TaskResourceRequests:
   ```
   +    val ereqs = new ExecutorResourceRequests()
   +    ereqs.cores(2).memory(4096, "m")
   +    ereqs.memoryOverhead(2048, "m").pysparkMemory(1024, "m")
   +    val treqs = new TaskResourceRequests()
   +    treqs.cpus(1)
   +
   +    rprof.require(treqs)
   +    rprof.require(ereqs)
    
   ```
   
   This leaves us the flexibility in the future to add things like .prefer to 
ResourceProfile.
   thoughts on this?  I'll probably push these changes shortly for you to take 
a look at the code. I can always role back if we don't like it.

----------------------------------------------------------------
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]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to