Uploaded image for project: 'Apache Tez'
  1. Apache Tez
  2. TEZ-1816

It is possible to receive START event when DAG is failed

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 0.6.0
    • None
    • None

    Description

      If DAG is failed in initialization, then it would goto failed. Then there will be a following START event from client.

      Attachments

        1. TEZ-1816.patch
          2 kB
          Jeff Zhang
        2. TEZ-1816-2.patch
          2 kB
          Jeff Zhang

        Activity

          zjffdu Jeff Zhang added a comment -

          Attach patch to ignore START event when vertex is in the state of FAILED.
          bikassaha, sseth, hitesh please help review .

          zjffdu Jeff Zhang added a comment - Attach patch to ignore START event when vertex is in the state of FAILED. bikassaha , sseth , hitesh please help review .
          sseth Siddharth Seth added a comment -

          +1 with one minor change. The test needs to wait for all transitions.

          +    dagWithCustomEdge.handle(new DAGEvent(dagWithCustomEdge.getID(), DAGEventType.DAG_START));
          +    Assert.assertEquals(DAGState.FAILED, dagWithCustomEdge.getState());
          

          should be

          +    dagWithCustomEdge.handle(new DAGEvent(dagWithCustomEdge.getID(), DAGEventType.DAG_START));
          dispatcher.await();
          +    Assert.assertEquals(DAGState.FAILED, dagWithCustomEdge.getState());
          
          sseth Siddharth Seth added a comment - +1 with one minor change. The test needs to wait for all transitions. + dagWithCustomEdge.handle( new DAGEvent(dagWithCustomEdge.getID(), DAGEventType.DAG_START)); + Assert.assertEquals(DAGState.FAILED, dagWithCustomEdge.getState()); should be + dagWithCustomEdge.handle( new DAGEvent(dagWithCustomEdge.getID(), DAGEventType.DAG_START)); dispatcher.await(); + Assert.assertEquals(DAGState.FAILED, dagWithCustomEdge.getState());
          zjffdu Jeff Zhang added a comment -

          sseth Is it necessary to add dispatcher.await() here ? Is it for any potential events sent after this handle method ?
          DAG.handle is a sync call, will trigger the state machine transition at once.

          zjffdu Jeff Zhang added a comment - sseth Is it necessary to add dispatcher.await() here ? Is it for any potential events sent after this handle method ? DAG.handle is a sync call, will trigger the state machine transition at once.
          sseth Siddharth Seth added a comment -

          In case of an internal error - which is what will be triggered without the patch - the ERROR event is sent to the async queue, so will not necessarily be processed inline.
          The modified test passes for me without the changes to DAGImpl - since the event in the queue is not processed - and the DAG stays in the FAILED state for the test.

          sseth Siddharth Seth added a comment - In case of an internal error - which is what will be triggered without the patch - the ERROR event is sent to the async queue, so will not necessarily be processed inline. The modified test passes for me without the changes to DAGImpl - since the event in the queue is not processed - and the DAG stays in the FAILED state for the test.
          zjffdu Jeff Zhang added a comment -

          Thanks, sseth, upload new patch, will commit soon.

          zjffdu Jeff Zhang added a comment - Thanks, sseth , upload new patch, will commit soon.
          zjffdu Jeff Zhang added a comment -

          Committed to master

          zjffdu Jeff Zhang added a comment - Committed to master
          hitesh Hitesh Shah added a comment -

          Closing jira as 0.6.0 is released.

          hitesh Hitesh Shah added a comment - Closing jira as 0.6.0 is released.

          People

            zjffdu Jeff Zhang
            zjffdu Jeff Zhang
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: