ayushtkn commented on code in PR #301:
URL: https://github.com/apache/tez/pull/301#discussion_r1251875365
##########
tez-api/src/main/java/org/apache/tez/serviceplugins/api/TaskSchedulerContext.java:
##########
@@ -84,6 +84,20 @@ void taskAllocated(Object task,
Object appCookie,
Container container);
+ /**
+ * Indicate to the framework that a container is being allocated.
+ *
+ * @param the actual container
+ */
+ void containerAllocated(Container container);
+
+ /**
+ * Indicate to the framework that a container is being reused:
+ * there is a task assigned to an already used container.
+ *
+ * @param the actual container
Review Comment:
should be
```
* @param container the actual container
```
##########
tez-dag/src/main/java/org/apache/tez/dag/app/rm/TaskSchedulerManager.java:
##########
@@ -951,6 +954,14 @@ public void dagCompleted() {
}
}
+ public int getAlreadyHeldContainersCount() {
+ int count = 0;
+ for (int i = 0 ; i < taskSchedulers.length ; i++) {
+ count += taskSchedulers[i].getTaskScheduler().getHeldContainersCount();
+ }
Review Comment:
enhanced for loop looks more fancy
```
for (TaskSchedulerWrapper taskScheduler : taskSchedulers) {
count += taskScheduler.getTaskScheduler().getHeldContainersCount();
}
```
##########
tez-api/src/main/java/org/apache/tez/serviceplugins/api/TaskScheduler.java:
##########
@@ -263,4 +265,19 @@ public abstract boolean deallocateTask(Object task,
boolean taskSucceeded,
*/
public abstract void dagComplete() throws ServicePluginException;
+ /**
+ * Get the number of held containers
+ * @throws ServicePluginException when the service runs into a fatal error
which it cannot handle.
+ * This will cause the app to shutdown.
+ */
+ public abstract int getHeldContainersCount();
Review Comment:
this doesn't throw this exception
##########
tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java:
##########
@@ -895,6 +898,12 @@ protected synchronized void handle(DAGAppMasterEvent
event) {
}
}
+ private void handleUsedContainersOnDagFinish(DAG dag) {
+ dag.incrementDagCounter(DAGCounter.TOTAL_CONTAINERS_USED,
Review Comment:
is it actually an increment or a set? mens just from the name point of view
+ can this have some leftovers due to failures + retires and stuff like that?
##########
tez-api/src/main/java/org/apache/tez/serviceplugins/api/TaskSchedulerContext.java:
##########
@@ -84,6 +84,20 @@ void taskAllocated(Object task,
Object appCookie,
Container container);
+ /**
+ * Indicate to the framework that a container is being allocated.
+ *
+ * @param the actual container
Review Comment:
this isn't correct:
```
* @param container the actual container
```
--
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.
To unsubscribe, e-mail: issues-unsubscr...@tez.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
