prasanthj commented on a change in pull request #1431:
URL: https://github.com/apache/hive/pull/1431#discussion_r476626802
##########
File path:
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskSchedulerService.java
##########
@@ -1487,6 +1492,14 @@ private SelectHostResult selectHost(TaskInfo request) {
return SELECT_HOST_RESULT_DELAYED_RESOURCES;
}
+ // When all nodes are busy, reset locality delay
+ if (activeNodesWithFreeSlots.isEmpty()) {
+ isCapacityFull.set(true);
+ if (request.localityDelayTimeout > 0 &&
isRequestedHostPresent(request)) {
+ request.resetLocalityDelayInfo();
+ }
+ }
Review comment:
isCapacityFull can be set to false in else condition here? instead of
trySchedulingPendingTasks.. just a minor readability improvement..
##########
File path:
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskSchedulerService.java
##########
@@ -1817,8 +1830,10 @@ protected void schedulePendingTasks() throws
InterruptedException {
Iterator<TaskInfo> taskIter = taskListAtPriority.iterator();
boolean scheduledAllAtPriority = true;
while (taskIter.hasNext()) {
- // TODO Optimization: Add a check to see if there's any capacity
available. No point in
- // walking through all active nodes, if they don't have potential
capacity.
+ // Early exit where there are no slots available
+ if (isCapacityFull.get()) {
Review comment:
This can be outer if condition? only if cluster capacity is available
run the task iterator.
##########
File path:
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskSchedulerService.java
##########
@@ -1390,7 +1395,7 @@ private SelectHostResult selectHost(TaskInfo request) {
boolean shouldDelayForLocality =
request.shouldDelayForLocality(schedulerAttemptTime);
LOG.debug("ShouldDelayForLocality={} for task={} on hosts={}",
shouldDelayForLocality,
request.task, requestedHostsDebugStr);
- if (requestedHosts != null && requestedHosts.length > 0) {
+ if (!isRequestedHostPresent(request)) {
Review comment:
the condition seems to have flipped here. is this expected?
##########
File path:
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskSchedulerService.java
##########
@@ -251,6 +251,7 @@ public void setError(Void v, Throwable t) {
private final Lock scheduleLock = new ReentrantLock();
private final Condition scheduleCondition = scheduleLock.newCondition();
+ private final AtomicBoolean isCapacityFull = new AtomicBoolean(false);
Review comment:
nit: to differentiate node vs cluster, rename this to
isClusterCapacityFull?
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
