[
https://issues.apache.org/jira/browse/HIVE-24061?focusedWorklogId=474425&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-474425
]
ASF GitHub Bot logged work on HIVE-24061:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 25/Aug/20 17:45
Start Date: 25/Aug/20 17:45
Worklog Time Spent: 10m
Work Description: 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:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 474425)
Time Spent: 20m (was: 10m)
> Improve llap task scheduling for better cache hit rate
> -------------------------------------------------------
>
> Key: HIVE-24061
> URL: https://issues.apache.org/jira/browse/HIVE-24061
> Project: Hive
> Issue Type: Improvement
> Reporter: Rajesh Balamohan
> Assignee: Rajesh Balamohan
> Priority: Major
> Labels: perfomance, pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> TaskInfo is initialized with the "requestTime and locality delay". When lots
> of vertices are in the same level, "taskInfo" details would be available
> upfront. By the time, it gets to scheduling, "requestTime + localityDelay"
> won't be higher than current time. Due to this, it misses scheduling delay
> details and ends up choosing random node. This ends up missing cache hits and
> reads data from remote storage.
> E.g Observed this pattern in Q75 of tpcds.
> Related lines of interest in scheduler:
> [https://github.com/apache/hive/blob/master/llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskSchedulerService.java
>
> |https://github.com/apache/hive/blob/master/llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskSchedulerService.java]
> {code:java}
> boolean shouldDelayForLocality =
> request.shouldDelayForLocality(schedulerAttemptTime);
> ..
> ..
> boolean shouldDelayForLocality(long schedulerAttemptTime) {
> return localityDelayTimeout > schedulerAttemptTime;
> }
> {code}
>
> Ideally, "localityDelayTimeout" should be adjusted based on it's first
> scheduling opportunity.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
