Details

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

      Description

      The query status synchronization on state machine always wait for time (event processing time) before status changes. If a lot of time are being processed to the event dispatcher, client api(getStatus, getProgress) will be waiting for changed event status. and query will be session timed out.

        Activity

        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user jinossy opened a pull request:

        https://github.com/apache/tajo/pull/99

        TAJO-985: Client API should be async

        The query status synchronization on state machine always wait for time (event processing time) before status changes. If a lot of time are being processed to the event dispatcher, client api(getStatus, getProgress) will be waiting for changed event status. and query will be session timed out.

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/jinossy/tajo TAJO-985

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/tajo/pull/99.patch

        To close this pull request, make a commit to your master/trunk branch
        with (at least) the following in the commit message:

        This closes #99


        commit 565744dea7ebcd80de100741465cab606fdad068
        Author: jinossy <jinossy@gmail.com>
        Date: 2014-07-29T08:54:00Z

        TAJO-985: Client API should be async

        commit e4eb0fe704a7499858ba4821910e75f1577c79a8
        Author: jinossy <jinossy@gmail.com>
        Date: 2014-07-30T02:50:58Z

        fixed wrong progress

        commit e6326d8b6dd26742de026dba1d0a4d34f2bc99cd
        Author: jinossy <jinossy@gmail.com>
        Date: 2014-07-30T02:57:20Z

        applied to web ui

        commit 59425a86fae13afb90cda6c91fbc2ce02bfdc8ed
        Author: jinossy <jinossy@gmail.com>
        Date: 2014-07-31T07:01:43Z

        added event executor


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user jinossy opened a pull request: https://github.com/apache/tajo/pull/99 TAJO-985 : Client API should be async The query status synchronization on state machine always wait for time (event processing time) before status changes. If a lot of time are being processed to the event dispatcher, client api(getStatus, getProgress) will be waiting for changed event status. and query will be session timed out. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jinossy/tajo TAJO-985 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/99.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #99 commit 565744dea7ebcd80de100741465cab606fdad068 Author: jinossy <jinossy@gmail.com> Date: 2014-07-29T08:54:00Z TAJO-985 : Client API should be async commit e4eb0fe704a7499858ba4821910e75f1577c79a8 Author: jinossy <jinossy@gmail.com> Date: 2014-07-30T02:50:58Z fixed wrong progress commit e6326d8b6dd26742de026dba1d0a4d34f2bc99cd Author: jinossy <jinossy@gmail.com> Date: 2014-07-30T02:57:20Z applied to web ui commit 59425a86fae13afb90cda6c91fbc2ce02bfdc8ed Author: jinossy <jinossy@gmail.com> Date: 2014-07-31T07:01:43Z added event executor
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/99#discussion_r15741540

        — Diff: tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java —
        @@ -487,7 +487,7 @@ private void waitForQueryCompleted(QueryId queryId) {
        while (true) {
        // TODO - configurable
        status = client.getQueryStatus(queryId);

        • if(status.getState() == QueryState.QUERY_MASTER_INIT || status.getState() == QueryState.QUERY_MASTER_LAUNCHED) {
            • End diff –

        The line is longer than 120 columns.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on a diff in the pull request: https://github.com/apache/tajo/pull/99#discussion_r15741540 — Diff: tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java — @@ -487,7 +487,7 @@ private void waitForQueryCompleted(QueryId queryId) { while (true) { // TODO - configurable status = client.getQueryStatus(queryId); if(status.getState() == QueryState.QUERY_MASTER_INIT || status.getState() == QueryState.QUERY_MASTER_LAUNCHED) { End diff – The line is longer than 120 columns.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/99#discussion_r15741545

        — Diff: tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java —
        @@ -487,7 +487,7 @@ private void waitForQueryCompleted(QueryId queryId) {
        while (true) {
        // TODO - configurable
        status = client.getQueryStatus(queryId);

        • if(status.getState() == QueryState.QUERY_MASTER_INIT || status.getState() == QueryState.QUERY_MASTER_LAUNCHED) {
          + if(status.getState() == QueryState.QUERY_NEW || status.getState() == QueryState.QUERY_MASTER_INIT || status.getState() == QueryState.QUERY_MASTER_LAUNCHED) {
            • End diff –

        The line is longer than 120 columns.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on a diff in the pull request: https://github.com/apache/tajo/pull/99#discussion_r15741545 — Diff: tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java — @@ -487,7 +487,7 @@ private void waitForQueryCompleted(QueryId queryId) { while (true) { // TODO - configurable status = client.getQueryStatus(queryId); if(status.getState() == QueryState.QUERY_MASTER_INIT || status.getState() == QueryState.QUERY_MASTER_LAUNCHED) { + if(status.getState() == QueryState.QUERY_NEW || status.getState() == QueryState.QUERY_MASTER_INIT || status.getState() == QueryState.QUERY_MASTER_LAUNCHED) { End diff – The line is longer than 120 columns.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jinossy commented on the pull request:

        https://github.com/apache/tajo/pull/99#issuecomment-51149887

        Thank you for the review. I reflected your comments.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jinossy commented on the pull request: https://github.com/apache/tajo/pull/99#issuecomment-51149887 Thank you for the review. I reflected your comments.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/99#discussion_r16035452

        — Diff: tajo-core/src/main/java/org/apache/tajo/master/querymaster/Query.java —
        @@ -337,15 +340,24 @@ public SubQuery getSubQuery(ExecutionBlockId id)

        { return this.subqueries.values(); }
        • public QueryState getState() {
        • readLock.lock();
        • try { - return stateMachine.getCurrentState(); - }

          finally {

        • readLock.unlock();
          + protected QueryState getState(boolean async) {
            • End diff –

        I think that the variable name ```async``` should be nonblocking.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on a diff in the pull request: https://github.com/apache/tajo/pull/99#discussion_r16035452 — Diff: tajo-core/src/main/java/org/apache/tajo/master/querymaster/Query.java — @@ -337,15 +340,24 @@ public SubQuery getSubQuery(ExecutionBlockId id) { return this.subqueries.values(); } public QueryState getState() { readLock.lock(); try { - return stateMachine.getCurrentState(); - } finally { readLock.unlock(); + protected QueryState getState(boolean async) { End diff – I think that the variable name ```async``` should be nonblocking.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/99#issuecomment-51736398

        The main idea looks good to me. It will mitigate lock contention problem.

        I have one suggestion. How about changing getState(boolean) and getState signatures into different names? Since the uses of them may be sensitive, we need to know what we invoke more apparently.

        I would like to suggest getSynchronizedState() and getState(). It's just a suggestion. The decision is up to you.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/99#issuecomment-51736398 The main idea looks good to me. It will mitigate lock contention problem. I have one suggestion. How about changing getState(boolean) and getState signatures into different names? Since the uses of them may be sensitive, we need to know what we invoke more apparently. I would like to suggest getSynchronizedState() and getState(). It's just a suggestion. The decision is up to you.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jinossy commented on the pull request:

        https://github.com/apache/tajo/pull/99#issuecomment-51737829

        @hyunsik good idea.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jinossy commented on the pull request: https://github.com/apache/tajo/pull/99#issuecomment-51737829 @hyunsik good idea.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jinossy commented on the pull request:

        https://github.com/apache/tajo/pull/99#issuecomment-51748431

        I’ve reflects your suggestion.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jinossy commented on the pull request: https://github.com/apache/tajo/pull/99#issuecomment-51748431 I’ve reflects your suggestion.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/99#issuecomment-52447449

        +1

        The patch looks nice to me. Ship it.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/99#issuecomment-52447449 +1 The patch looks nice to me. Ship it.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/99#issuecomment-52447530

        The word 'async' was changed during the review. So, I think that the issue title also should be changed to proper name. Could you change the title before committing?

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/99#issuecomment-52447530 The word 'async' was changed during the review. So, I think that the issue title also should be changed to proper name. Could you change the title before committing?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jinossy commented on the pull request:

        https://github.com/apache/tajo/pull/99#issuecomment-52450810

        Thank you for the review.
        I've just committed it.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jinossy commented on the pull request: https://github.com/apache/tajo/pull/99#issuecomment-52450810 Thank you for the review. I've just committed it.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jinossy closed the pull request at:

        https://github.com/apache/tajo/pull/99

        Show
        githubbot ASF GitHub Bot added a comment - Github user jinossy closed the pull request at: https://github.com/apache/tajo/pull/99
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #337 (See https://builds.apache.org/job/Tajo-master-build/337/)
        TAJO-985: Client API should be non-blocking. (jinho) (jhkim: rev 60858b25c46fceedf70b8583f1d7ef1383a19c6b)

        • tajo-core/src/main/java/org/apache/tajo/master/querymaster/QueryMasterTask.java
        • CHANGES
        • tajo-core/src/main/java/org/apache/tajo/worker/TajoWorkerClientService.java
        • tajo-core/src/main/java/org/apache/tajo/master/querymaster/QueryMaster.java
        • tajo-core/src/test/java/org/apache/tajo/client/TestTajoClient.java
        • tajo-core/src/main/java/org/apache/tajo/master/querymaster/SubQuery.java
        • tajo-core/src/main/java/org/apache/tajo/master/querymaster/Query.java
        • tajo-client/src/main/java/org/apache/tajo/client/TajoClient.java
        • tajo-core/src/main/java/org/apache/tajo/worker/TajoResourceAllocator.java
        • tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java
        • tajo-client/src/main/java/org/apache/tajo/client/TajoAdmin.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #337 (See https://builds.apache.org/job/Tajo-master-build/337/ ) TAJO-985 : Client API should be non-blocking. (jinho) (jhkim: rev 60858b25c46fceedf70b8583f1d7ef1383a19c6b) tajo-core/src/main/java/org/apache/tajo/master/querymaster/QueryMasterTask.java CHANGES tajo-core/src/main/java/org/apache/tajo/worker/TajoWorkerClientService.java tajo-core/src/main/java/org/apache/tajo/master/querymaster/QueryMaster.java tajo-core/src/test/java/org/apache/tajo/client/TestTajoClient.java tajo-core/src/main/java/org/apache/tajo/master/querymaster/SubQuery.java tajo-core/src/main/java/org/apache/tajo/master/querymaster/Query.java tajo-client/src/main/java/org/apache/tajo/client/TajoClient.java tajo-core/src/main/java/org/apache/tajo/worker/TajoResourceAllocator.java tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java tajo-client/src/main/java/org/apache/tajo/client/TajoAdmin.java

          People

          • Assignee:
            jhkim Jinho Kim
            Reporter:
            jhkim Jinho Kim
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development