[
https://issues.apache.org/jira/browse/TEZ-714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14486038#comment-14486038
]
Bikas Saha edited comment on TEZ-714 at 4/8/15 9:06 PM:
--------------------------------------------------------
abortVertex() should probably be called for all non-succeeded states? Similar
to DAGImpl. Otherwise TaskCompletedAfterVertexSuccessTransition will have a
regression.
{code} case ERROR:
addDiagnostic("Vertex: " + logIdentifier + " error due to:" +
terminationCause);
if (!StringUtils.isEmpty(diag)) {
addDiagnostic(diag);
}
eventHandler.handle(new DAGEvent(getDAGId(),
DAGEventType.INTERNAL_ERROR));
try {
logJobHistoryVertexFailedEvent(finalState);
} catch (IOException e) {
LOG.error("Failed to send vertex finished event to recovery", e);
}
break;
case KILLED:
case FAILED:
addDiagnostic("Vertex " + logIdentifier + " killed/failed due to:" +
terminationCause);
if (!StringUtils.isEmpty(diag)) {
addDiagnostic(diag);
}
abortVertex(VertexStatus.State.valueOf(finalState.name()));
{code}
testCommitOutputOnVertexSuccess() still has
{code} // both committers fail
DAG dag4 = createDAG("testDAGBothCommitsFail", false, true); <<< true/true
{code}
Mis-written comment? v3 is killed due to...
{code} // killed is failed due to the commit failure of the vertex group
(v1,v2)
Assert.assertEquals(VertexState.KILLED, v3.getState());
Assert.assertEquals(VertexTerminationCause.OTHER_VERTEX_FAILURE,
{code}
The test is not checking/verifying that one of the commits was indeed not
scheduled on the threadpool and cancelled before that. Maybe set a flag when
the callable is scheduled and check that the flag was not set. Also perhaps set
a flag when interrupted exception is caught when the scheduled event got
canceled.
{code}
// test commit will be canceled no matter it is started or still in the
threadpool
@Test(timeout = 5000)
public void testCommitCanceled() throws Exception {
{code}
Please cleanup the findbugs warnings.
Spoke offline with [~hitesh] about aborting all output after any failure and it
seems like the right thing to do.
was (Author: bikassaha):
abortVertex() should probably be called for all non-succeeded states? Other
TaskCompletedAfterVertexSuccessTransition will have a regression.
{code} case ERROR:
addDiagnostic("Vertex: " + logIdentifier + " error due to:" +
terminationCause);
if (!StringUtils.isEmpty(diag)) {
addDiagnostic(diag);
}
eventHandler.handle(new DAGEvent(getDAGId(),
DAGEventType.INTERNAL_ERROR));
try {
logJobHistoryVertexFailedEvent(finalState);
} catch (IOException e) {
LOG.error("Failed to send vertex finished event to recovery", e);
}
break;
case KILLED:
case FAILED:
addDiagnostic("Vertex " + logIdentifier + " killed/failed due to:" +
terminationCause);
if (!StringUtils.isEmpty(diag)) {
addDiagnostic(diag);
}
abortVertex(VertexStatus.State.valueOf(finalState.name()));
{code}
testCommitOutputOnVertexSuccess() still has
{code} // both committers fail
DAG dag4 = createDAG("testDAGBothCommitsFail", false, true); <<< true/true
{code}
Mis-written comment? v3 is killed due to...
{code} // killed is failed due to the commit failure of the vertex group
(v1,v2)
Assert.assertEquals(VertexState.KILLED, v3.getState());
Assert.assertEquals(VertexTerminationCause.OTHER_VERTEX_FAILURE,
{code}
The test is not checking/verifying that one of the commits was indeed not
scheduled on the threadpool and cancelled before that. Maybe set a flag when
the callable is scheduled and check that the flag was not set. Also perhaps set
a flag when interrupted exception is caught when the scheduled event got
canceled.
{code}
// test commit will be canceled no matter it is started or still in the
threadpool
@Test(timeout = 5000)
public void testCommitCanceled() throws Exception {
{code}
Please cleanup the findbugs warnings.
Spoke offline with [~hitesh] about aborting all output after any failure and it
seems like the right thing to do.
> OutputCommitters should not run in the main AM dispatcher thread
> ----------------------------------------------------------------
>
> Key: TEZ-714
> URL: https://issues.apache.org/jira/browse/TEZ-714
> Project: Apache Tez
> Issue Type: Improvement
> Reporter: Siddharth Seth
> Assignee: Jeff Zhang
> Priority: Critical
> Attachments: DAG_2.pdf, TEZ-714-1.patch, TEZ-714-10.patch,
> TEZ-714-2.patch, TEZ-714-3.patch, TEZ-714-4.patch, TEZ-714-5.patch,
> TEZ-714-6.patch, TEZ-714-7.patch, TEZ-714-8.patch, TEZ-714-9.patch,
> Vertex_2.pdf
>
>
> Follow up jira from TEZ-41.
> 1) If there's multiple OutputCommitters on a Vertex, they can be run in
> parallel.
> 2) Running an OutputCommitter in the main thread blocks all other event
> handling, w.r.t the DAG, and causes the event queue to back up.
> 3) This should also cover shared commits that happen in the DAG.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
