[ 
https://issues.apache.org/jira/browse/MAPREDUCE-5583?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14329658#comment-14329658
 ] 

Junping Du commented on MAPREDUCE-5583:
---------------------------------------

Thanks [~jlowe] for updating the patch. A few comments here:
I see we checked three times of canAssignMaps() In assignMapsWithLocality() in 
while loop. Can we simply return in the beginning of method if this is false.

{code}
+  protected void setRequestLimit(Priority priority, Resource capability,
+      Integer limit) {
+    if (limit == null || limit < 0) {
+      limit = Integer.MAX_VALUE;
+    }
{code}
Any special reason to use Integer instead of int for limit? If no, I think 
using int could be slightly better, then we can use "if (limit <= 0)". Isn't it?

{code}
+  private void applyRequestLimits() {
+    Iterator<ResourceRequest> iter = requestLimits.values().iterator();
+    while (iter.hasNext()) {
+      ResourceRequest reqLimit = iter.next();
+      int limit = reqLimit.getNumContainers();
+      Map<String, Map<Resource, ResourceRequest>> remoteRequests =
+          remoteRequestsTable.get(reqLimit.getPriority());
+      Map<Resource, ResourceRequest> reqMap = (remoteRequests != null)
+          ? remoteRequests.get(ResourceRequest.ANY) : null;
+      ResourceRequest req = (reqMap != null)
+          ? reqMap.get(reqLimit.getCapability()) : null;
+      if (req == null) {
+        continue;
+      }
+      // update an existing ask or send a new one if updating
+      if (ask.remove(req) || requestLimitsToUpdate.contains(req)) {
+        ResourceRequest newReq = req.getNumContainers() > limit
+            ? reqLimit : req;
+        ask.add(newReq);
+        LOG.info("Applying ask limit of " + newReq.getNumContainers()
+            + " for priority:" + reqLimit.getPriority()
+            + " and capability:" + reqLimit.getCapability());
+      }
+      if (limit == Integer.MAX_VALUE) {
+        iter.remove();
+      }
+    }
+    requestLimitsToUpdate.clear();
+  }
{code}
I think we are filtering ask here (to make sure don't asking too much 
containers that beyond the limit) before making remote request to RM. If so, 
why do we only filter request with "Locality.ANY"? Local node + local rack 
request number could also exceed this limit. Isn't it? I think we should filter 
the requests (start from Locality.ANY) and could extend to RACK_LOCAL, then 
NODE_LOCAL. Thoughts? 
In addition, it sounds like we use Integer.MAX_VALUE as a mark for no limit. 
Can we check it earlier and return?

> Ability to limit running map and reduce tasks
> ---------------------------------------------
>
>                 Key: MAPREDUCE-5583
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-5583
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: mr-am, mrv2
>    Affects Versions: 0.23.9, 2.1.1-beta
>            Reporter: Jason Lowe
>            Assignee: Jason Lowe
>         Attachments: MAPREDUCE-5583-branch2.4.1.patch, 
> MAPREDUCE-5583v1.patch, MAPREDUCE-5583v2.patch, MAPREDUCE-5583v3.patch
>
>
> It would be nice if users could specify a limit to the number of map or 
> reduce tasks that are running simultaneously.  Occasionally users are 
> performing operations in tasks that can lead to DDoS scenarios if too many 
> tasks run simultaneously (e.g.: accessing a database, web service, etc.).  
> Having the ability to throttle the number of tasks simultaneously running 
> would provide users a way to mitigate issues with too many tasks on a large 
> cluster attempting to access a serivce at any one time.
> This is similar to the functionality requested by MAPREDUCE-224 and 
> implemented by HADOOP-3412 but was dropped in mrv2.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to