[
https://issues.apache.org/jira/browse/TEZ-2707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14692914#comment-14692914
]
Bikas Saha commented on TEZ-2707:
---------------------------------
We need to start reviewing changes before committing them. scheduledTime is
correct in master for the purpose for which it was introduced - to capture when
the attempt was created (either as part of initial scheduling by vertex manager
or due to a retry). Which is why it was set in the constructor. By the time
ScheduleTaskattemptTransition is called, like you correctly observe, a large
time might pass because of say delays in the dag scheduler or some other place.
So that time needs to be accounted for because it has effectively increased the
critical path for this attempt. So if we need to record that time, we need
another variable with some other variation of the scheduled word :P. But the
scheduledTime as currently exists in master is correct. If we need to record
the ScheduleTaskattemptTransition time then we should create a new timestamp
for it - e.g dagScheduled or priorityDecided timestamp. Or rename the one in
master to creationTime. Of course we will soon need another scheduled time when
the actual scheduler matches the container to the attempt to find out how long
it took for that operation :P
{code}---
tez-dag/src/main/java/org/apache/tez/dag/app/dag/impl/TaskAttemptImpl.java
+++ tez-dag/src/main/java/org/apache/tez/dag/app/dag/impl/TaskAttemptImpl.java
@@ -133,7 +133,6 @@ public class TaskAttemptImpl implements TaskAttempt,
protected final AppContext appContext;
private final TaskHeartbeatHandler taskHeartbeatHandler;
private long launchTime = 0;
- private long scheduleTime = 0;
private long finishTime = 0;
private String trackerName;
private int httpPort;
@@ -437,7 +436,6 @@ public class TaskAttemptImpl implements TaskAttempt,
this.task = task;
this.vertex = this.task.getVertex();
this.schedulingCausalTA = schedulingCausalTA;
- this.scheduledTime = clock.getTime();
this.reportedStatus = new TaskAttemptStatus(this.attemptId);
initTaskAttemptStatus(reportedStatus);
@@ -674,7 +672,7 @@ public class TaskAttemptImpl implements TaskAttempt,
public long getScheduleTime() {
readLock.lock();
try {
- return scheduleTime;
+ return scheduledTime;
} finally {
readLock.unlock();
}
@@ -1040,7 +1038,7 @@ public class TaskAttemptImpl implements TaskAttempt,
public TaskAttemptStateInternal transition(TaskAttemptImpl ta,
TaskAttemptEvent event) {
TaskAttemptEventSchedule scheduleEvent = (TaskAttemptEventSchedule)
event;
- ta.scheduleTime = ta.clock.getTime();
+ ta.scheduledTime = ta.clock.getTime();
// TODO Creating the remote task here may not be required in case of
// recovery.{code}
> Fix comments from reviews - part 2
> ----------------------------------
>
> Key: TEZ-2707
> URL: https://issues.apache.org/jira/browse/TEZ-2707
> Project: Apache Tez
> Issue Type: Sub-task
> Affects Versions: TEZ-2003
> Reporter: Siddharth Seth
> Assignee: Siddharth Seth
> Fix For: TEZ-2003
>
> Attachments: TEZ-2707.1.txt
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
