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

In the case of very quick query, client can't get query status.

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.8.0, 0.9.0
    • Component/s: None
    • Labels:
      None

      Description

      See the title. TajoClient calls getQueryStatus() to TajoMaster or QueryMaster every 500ms. If a query is ended within 500ms, TajoClient can't get query status from TajoMaster. The cause of this problem is that TajoMaster get a status from QueryInProgress.

      1. TAJO-706_3.patch
        8 kB
        Hyunsik Choi
      2. TAJO-706_2.patch
        7 kB
        Hyoungjun Kim
      3. TAJO-706.patch
        5 kB
        Hyoungjun Kim

        Activity

        Hide
        jihoonson Jihoon Son added a comment -

        +1.
        This is mandatory issue.

        Show
        jihoonson Jihoon Son added a comment - +1. This is mandatory issue.
        Hide
        hjkim Hyoungjun Kim added a comment -

        I fixed this bug.
        In addition, TajoDump were also some improvements in this patch. Even when some table is failed, other tables is successfully dumped.
        Please review this patch.

        Show
        hjkim Hyoungjun Kim added a comment - I fixed this bug. In addition, TajoDump were also some improvements in this patch. Even when some table is failed, other tables is successfully dumped. Please review this patch.
        Hide
        tajoqa Tajo QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12636698/TAJO-706.patch
        against master revision 4209b83.

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

        +1 checkstyle. The patch generated 0 code style errors.

        -1 findbugs. The patch appears to introduce 191 new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in tajo-client tajo-core/tajo-core-backend.

        Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/259//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/259//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/259//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core-backend.html
        Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/259//console

        This message is automatically generated.

        Show
        tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12636698/TAJO-706.patch against master revision 4209b83. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 191 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-client tajo-core/tajo-core-backend. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/259//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/259//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/259//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core-backend.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/259//console This message is automatically generated.
        Hide
        jihoonson Jihoon Son added a comment -

        Hyoungjun Kim, thanks for your contribution.

        This patch looks good to me, and passed all the unit tests.
        But, if you add a test, the patch would be great!
        Would you add an unit test?

        Thanks,
        Jihoon

        Show
        jihoonson Jihoon Son added a comment - Hyoungjun Kim , thanks for your contribution. This patch looks good to me, and passed all the unit tests. But, if you add a test, the patch would be great! Would you add an unit test? Thanks, Jihoon
        Hide
        jihoonson Jihoon Son added a comment -

        Hyoungjun Kim, sorry for burdening you, but I have one more suggestion.

        Nowadays, I'm designing a program using TajoClient. To get the query result, I adopt the exactly same way with TajoCli, but, sometimes, TajoClient.getResultResponse() doesn't work with emitting the following message.

        2014-03-26 11:25:07,839 WARN  client.TajoClient (TajoClient.java:getResultResponse(296)) - No Connection to QueryMaster for q_1395800666626_0001
        

        I think that this problem is also caused by the request to the halted query master.
        However, many applications generally require to get query results even after the query is finished, and, in my opinion, we should support it.

        I think that this problem can be easily handled by the same way in your patch.
        Would you add this feature in the next patch, please?

        Thanks,
        Jihoon

        Show
        jihoonson Jihoon Son added a comment - Hyoungjun Kim , sorry for burdening you, but I have one more suggestion. Nowadays, I'm designing a program using TajoClient. To get the query result, I adopt the exactly same way with TajoCli, but, sometimes, TajoClient.getResultResponse() doesn't work with emitting the following message. 2014-03-26 11:25:07,839 WARN client.TajoClient (TajoClient.java:getResultResponse(296)) - No Connection to QueryMaster for q_1395800666626_0001 I think that this problem is also caused by the request to the halted query master. However, many applications generally require to get query results even after the query is finished, and, in my opinion, we should support it. I think that this problem can be easily handled by the same way in your patch. Would you add this feature in the next patch, please? Thanks, Jihoon
        Hide
        hjkim Hyoungjun Kim added a comment -

        Thanks Jihoon.
        I added a testcase for this issue in this patch.
        "No Connection to QueryMaster" message not reproduce. Please check after applying this patch.

        Show
        hjkim Hyoungjun Kim added a comment - Thanks Jihoon. I added a testcase for this issue in this patch. "No Connection to QueryMaster" message not reproduce. Please check after applying this patch.
        Hide
        tajoqa Tajo QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12637041/TAJO-706_2.patch
        against master revision e06ffa9.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

        +1 checkstyle. The patch generated 0 code style errors.

        -1 findbugs. The patch appears to introduce 196 new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in tajo-client tajo-core/tajo-core-backend.

        Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/268//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/268//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/268//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core-backend.html
        Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/268//console

        This message is automatically generated.

        Show
        tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12637041/TAJO-706_2.patch against master revision e06ffa9. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 196 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-client tajo-core/tajo-core-backend. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/268//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/268//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/268//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core-backend.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/268//console This message is automatically generated.
        Hide
        jihoonson Jihoon Son added a comment -

        Thanks, Hyoungjun Kim!

        I'll review the patch, and check the aforementioned problem after applying the patch.

        Show
        jihoonson Jihoon Son added a comment - Thanks, Hyoungjun Kim ! I'll review the patch, and check the aforementioned problem after applying the patch.
        Hide
        hyunsik Hyunsik Choi added a comment -

        I'm sorry for the late review.

        I just added some comments and timeout annotation in the patch. I believe that the comments make the changes more comprehensive.

        Also, in my opinion, the patch looks good to me. The main problem is that the finished query status cannot be obtained from getQueryStatus(). The patch exactly fixes the problem. I think that the unit test verifies the case well. Here is my +1.

        As far as I know, Jihoon has faced this problem in his application. So, his real test would be helpful to ensure that this change correctly fixes the problem.

        Show
        hyunsik Hyunsik Choi added a comment - I'm sorry for the late review. I just added some comments and timeout annotation in the patch. I believe that the comments make the changes more comprehensive. Also, in my opinion, the patch looks good to me. The main problem is that the finished query status cannot be obtained from getQueryStatus(). The patch exactly fixes the problem. I think that the unit test verifies the case well. Here is my +1. As far as I know, Jihoon has faced this problem in his application. So, his real test would be helpful to ensure that this change correctly fixes the problem.
        Hide
        tajoqa Tajo QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12637713/TAJO-706_3.patch
        against master revision bbbf21d.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

        +1 checkstyle. The patch generated 0 code style errors.

        -1 findbugs. The patch appears to introduce 198 new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in tajo-client tajo-core/tajo-core-backend.

        Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/285//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/285//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core-backend.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/285//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-client.html
        Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/285//console

        This message is automatically generated.

        Show
        tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12637713/TAJO-706_3.patch against master revision bbbf21d. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 198 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-client tajo-core/tajo-core-backend. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/285//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/285//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core-backend.html Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/285//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-client.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/285//console This message is automatically generated.
        Hide
        hyunsik Hyunsik Choi added a comment -

        I discussed hyoungjun's latest patch with Jihoon in offline manner. He said that the patch already worked well in real application. I'll commit it soon.

        Show
        hyunsik Hyunsik Choi added a comment - I discussed hyoungjun's latest patch with Jihoon in offline manner. He said that the patch already worked well in real application. I'll commit it soon.
        Hide
        hyunsik Hyunsik Choi added a comment -

        committed it to master and branch-0.8.0. Thanks all!

        Show
        hyunsik Hyunsik Choi added a comment - committed it to master and branch-0.8.0. Thanks all!
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #147 (See https://builds.apache.org/job/Tajo-master-build/147/)
        TAJO-706: In the case of very quick query, client can't get query status. (hyoungjunkim via hyunsik) (hyunsik: rev 2a2f89a2838b21e1820c33f3136d1aae9b4b89e1)

        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterClientService.java
        • tajo-client/src/main/java/org/apache/tajo/client/TajoDump.java
        • tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/client/TestTajoClient.java
        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryJobManager.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #147 (See https://builds.apache.org/job/Tajo-master-build/147/ ) TAJO-706 : In the case of very quick query, client can't get query status. (hyoungjunkim via hyunsik) (hyunsik: rev 2a2f89a2838b21e1820c33f3136d1aae9b4b89e1) tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterClientService.java tajo-client/src/main/java/org/apache/tajo/client/TajoDump.java tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/client/TestTajoClient.java tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryJobManager.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Tajo-0.8.0-build #51 (See https://builds.apache.org/job/Tajo-0.8.0-build/51/)
        TAJO-706: In the case of very quick query, client can't get query status. (hyoungjunkim via hyunsik) (hyunsik: rev c7e361676470e823b2e2718f98bfd05ab35ea992)

        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryJobManager.java
        • tajo-client/src/main/java/org/apache/tajo/client/TajoDump.java
        • tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterClientService.java
        • tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/client/TestTajoClient.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Tajo-0.8.0-build #51 (See https://builds.apache.org/job/Tajo-0.8.0-build/51/ ) TAJO-706 : In the case of very quick query, client can't get query status. (hyoungjunkim via hyunsik) (hyunsik: rev c7e361676470e823b2e2718f98bfd05ab35ea992) tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryJobManager.java tajo-client/src/main/java/org/apache/tajo/client/TajoDump.java tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterClientService.java tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/client/TestTajoClient.java

          People

          • Assignee:
            hjkim Hyoungjun Kim
            Reporter:
            hjkim Hyoungjun Kim
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development