Uploaded image for project: 'Tajo'
  1. Tajo
  2. TAJO-325

QueryState.NEW and QueryState.INIT should be combined into one state

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.8.0
    • Fix Version/s: 0.8.0
    • Component/s: QueryMaster
    • Labels:
      None

      Description

      From the diagram uploaded at TAJO-320, we know that in the beginning of a Query. It first in New state, and instantly become INIT state because QueryMasterTask generate INIT and START event in succession.

            queryTaskContext.getEventHandler().handle(new QueryEvent(queryId,
                QueryEventType.INIT));
            queryTaskContext.getEventHandler().handle(new QueryEvent(queryId,
                QueryEventType.START));
      

      There is no gap between those two event, and the init transition in Query show that that transition will do nothing except log the start time. It couldn't transit to FAILED state since there's no condition to FAILED.

        static class InitTransition
            implements MultipleArcTransition<Query, QueryEvent, QueryState> {
      
          @Override
          public QueryState transition(Query query, QueryEvent queryEvent) {
            query.setStartTime();
            //query.context.setState(QueryState.QUERY_INIT);
            return QueryState.QUERY_INIT;
          }
        }
      

      For simplicity, I suggest combine those two state into one. When I was writing code on TAJO-305, I came across the problem I need to deal with those two state and found that I need to write some useless transitions for one of the two states.

        Activity

        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-trunk-postcommit #566 (See https://builds.apache.org/job/Tajo-trunk-postcommit/566/)
        TAJO-325: QueryState.NEW and QueryState.INIT should be combined into one state. (Min Zhou via hyunsik) (hyunsik: https://git-wip-us.apache.org/repos/asf?p=incubator-tajo.git&a=commit&h=6565d2bf1e0f9170a318e06553b4c5c70ee0b642)

        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/event/QueryEventType.java
        • CHANGES.txt
        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/Query.java
        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/client/TajoClient.java
        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMasterTask.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-trunk-postcommit #566 (See https://builds.apache.org/job/Tajo-trunk-postcommit/566/ ) TAJO-325 : QueryState.NEW and QueryState.INIT should be combined into one state. (Min Zhou via hyunsik) (hyunsik: https://git-wip-us.apache.org/repos/asf?p=incubator-tajo.git&a=commit&h=6565d2bf1e0f9170a318e06553b4c5c70ee0b642 ) tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/event/QueryEventType.java CHANGES.txt tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/Query.java tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/client/TajoClient.java tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMasterTask.java
        Hide
        hyunsik Hyunsik Choi added a comment -

        Committed the patch to master branch. Thank you for your contribution!

        Show
        hyunsik Hyunsik Choi added a comment - Committed the patch to master branch. Thank you for your contribution!
        Hide
        hyunsik Hyunsik Choi added a comment -

        +1

        I reviewed the patch. The patch looks good, and passes all unit tests and rat check.

        Show
        hyunsik Hyunsik Choi added a comment - +1 I reviewed the patch. The patch looks good, and passes all unit tests and rat check.
        Hide
        coderplay Min Zhou added a comment -

        Attached a patch. Intentionally remain the INIT state in tajo_protos.proto in order to keep compatible with the client.

        Show
        coderplay Min Zhou added a comment - Attached a patch. Intentionally remain the INIT state in tajo_protos.proto in order to keep compatible with the client.
        Hide
        hyunsik Hyunsik Choi added a comment -

        +1 for your suggestion. This was a temporary solution for recording query initialization time. I also think that two states should be merged into one.

        Show
        hyunsik Hyunsik Choi added a comment - +1 for your suggestion. This was a temporary solution for recording query initialization time. I also think that two states should be merged into one.

          People

          • Assignee:
            coderplay Min Zhou
            Reporter:
            coderplay Min Zhou
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development