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

Separate query and ddl execution codes from GlobalEngine

    Details

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

      Description

      This is a code cleanup issue. The patch separates query execution, ddl execution, and hooks from GlobalEngine.

      1. TAJO-1261.patch
        84 kB
        Hyunsik Choi

        Activity

        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #515 (See https://builds.apache.org/job/Tajo-master-build/515/)
        TAJO-1261: Separate query and ddl execution codes from GlobalEngine. (hyunsik: rev a4c3484232e139d4d21d0cfb9c31e5d784de652b)

        • tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/InsertIntoHook.java
        • tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/CreateTableHook.java
        • tajo-core/src/main/java/org/apache/tajo/master/TajoMaster.java
        • tajo-core/src/main/java/org/apache/tajo/master/exec/DDLExecutor.java
        • tajo-core/src/main/java/org/apache/tajo/master/GlobalEngine.java
        • tajo-core/src/main/java/org/apache/tajo/master/querymaster/QueryMasterTask.java
        • tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/DistributedQueryHook.java
        • tajo-core/src/main/java/org/apache/tajo/master/exec/QueryExecutor.java
        • tajo-core/src/main/java/org/apache/tajo/master/TajoMasterClientService.java
        • CHANGES
        • tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/DistributedQueryHookManager.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #515 (See https://builds.apache.org/job/Tajo-master-build/515/ ) TAJO-1261 : Separate query and ddl execution codes from GlobalEngine. (hyunsik: rev a4c3484232e139d4d21d0cfb9c31e5d784de652b) tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/InsertIntoHook.java tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/CreateTableHook.java tajo-core/src/main/java/org/apache/tajo/master/TajoMaster.java tajo-core/src/main/java/org/apache/tajo/master/exec/DDLExecutor.java tajo-core/src/main/java/org/apache/tajo/master/GlobalEngine.java tajo-core/src/main/java/org/apache/tajo/master/querymaster/QueryMasterTask.java tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/DistributedQueryHook.java tajo-core/src/main/java/org/apache/tajo/master/exec/QueryExecutor.java tajo-core/src/main/java/org/apache/tajo/master/TajoMasterClientService.java CHANGES tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/DistributedQueryHookManager.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Tajo-master-CODEGEN-build #155 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/155/)
        TAJO-1261: Separate query and ddl execution codes from GlobalEngine. (hyunsik: rev a4c3484232e139d4d21d0cfb9c31e5d784de652b)

        • tajo-core/src/main/java/org/apache/tajo/master/querymaster/QueryMasterTask.java
        • tajo-core/src/main/java/org/apache/tajo/master/exec/QueryExecutor.java
        • tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/InsertIntoHook.java
        • tajo-core/src/main/java/org/apache/tajo/master/GlobalEngine.java
        • tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/DistributedQueryHook.java
        • tajo-core/src/main/java/org/apache/tajo/master/TajoMaster.java
        • CHANGES
        • tajo-core/src/main/java/org/apache/tajo/master/exec/DDLExecutor.java
        • tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/DistributedQueryHookManager.java
        • tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/CreateTableHook.java
        • tajo-core/src/main/java/org/apache/tajo/master/TajoMasterClientService.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #155 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/155/ ) TAJO-1261 : Separate query and ddl execution codes from GlobalEngine. (hyunsik: rev a4c3484232e139d4d21d0cfb9c31e5d784de652b) tajo-core/src/main/java/org/apache/tajo/master/querymaster/QueryMasterTask.java tajo-core/src/main/java/org/apache/tajo/master/exec/QueryExecutor.java tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/InsertIntoHook.java tajo-core/src/main/java/org/apache/tajo/master/GlobalEngine.java tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/DistributedQueryHook.java tajo-core/src/main/java/org/apache/tajo/master/TajoMaster.java CHANGES tajo-core/src/main/java/org/apache/tajo/master/exec/DDLExecutor.java tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/DistributedQueryHookManager.java tajo-core/src/main/java/org/apache/tajo/master/exec/prehook/CreateTableHook.java tajo-core/src/main/java/org/apache/tajo/master/TajoMasterClientService.java
        Hide
        hyunsik Hyunsik Choi added a comment -

        committed.

        Show
        hyunsik Hyunsik Choi added a comment - committed.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

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

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

        Github user jihoonson commented on the pull request:

        https://github.com/apache/tajo/pull/312#issuecomment-67804653

        Thanks for your understanding.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/312#issuecomment-67804653 Thanks for your understanding.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/312#issuecomment-67804360

        Hi @jihoonson,

        You are right. They can be removed now. In the patch, InsertHook did some special work to rewrite a logical plan. So, the hook was required. But, it doesn't now. I'll remove it and then commit the patch. Thank you for the comment.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/312#issuecomment-67804360 Hi @jihoonson, You are right. They can be removed now. In the patch, InsertHook did some special work to rewrite a logical plan. So, the hook was required. But, it doesn't now. I'll remove it and then commit the patch. Thank you for the comment.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on the pull request:

        https://github.com/apache/tajo/pull/312#issuecomment-67761407

        Sorry @ykrips @hyunsik,
        even though +1 is given, I would like to make a trivial suggestion.
        I have left some comments.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/312#issuecomment-67761407 Sorry @ykrips @hyunsik, even though +1 is given, I would like to make a trivial suggestion. I have left some comments.
        Hide
        githubbot ASF GitHub Bot added a comment -

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

        https://github.com/apache/tajo/pull/312#discussion_r22146002

        — Diff: tajo-core/src/main/java/org/apache/tajo/master/querymaster/QueryMasterTask.java —
        @@ -390,8 +392,8 @@ public synchronized void startQuery() {

        optimizer.optimize(queryContext, plan);

        • GlobalEngine.DistributedQueryHookManager hookManager = new GlobalEngine.DistributedQueryHookManager();
        • hookManager.addHook(new GlobalEngine.InsertHook());
          + DistributedQueryHookManager hookManager = new DistributedQueryHookManager();
          + hookManager.addHook(new InsertIntoHook());
            • End diff –

        Both QueryExecutor and QueryMasterTask have the DistributedQueryHookManager.
        So, it would be better to maintain it in only one module.
        Since this is not a big change, I think that we can fix this in this issue.

        IMHO, QueryMasterTask is the proper one.
        What do you think?

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/312#discussion_r22146002 — Diff: tajo-core/src/main/java/org/apache/tajo/master/querymaster/QueryMasterTask.java — @@ -390,8 +392,8 @@ public synchronized void startQuery() { optimizer.optimize(queryContext, plan); GlobalEngine.DistributedQueryHookManager hookManager = new GlobalEngine.DistributedQueryHookManager(); hookManager.addHook(new GlobalEngine.InsertHook()); + DistributedQueryHookManager hookManager = new DistributedQueryHookManager(); + hookManager.addHook(new InsertIntoHook()); End diff – Both QueryExecutor and QueryMasterTask have the DistributedQueryHookManager. So, it would be better to maintain it in only one module. Since this is not a big change, I think that we can fix this in this issue. IMHO, QueryMasterTask is the proper one. What do you think?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user ykrips commented on the pull request:

        https://github.com/apache/tajo/pull/312#issuecomment-67759947

        Thank you for your guide. I need to much be familiar to Apache process.
        I would like to give +1 to this issue. These two executors might be helpful when we extend functions on global engine.

        Show
        githubbot ASF GitHub Bot added a comment - Github user ykrips commented on the pull request: https://github.com/apache/tajo/pull/312#issuecomment-67759947 Thank you for your guide. I need to much be familiar to Apache process. I would like to give +1 to this issue. These two executors might be helpful when we extend functions on global engine.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/312#issuecomment-67759656

        Thank you for your comment. If you think that some patch is acceptable, you can give one +1 for the patch. Then, the patch can be committed to a targeted branch. The Apache voting process is described at http://www.apache.org/foundation/voting.html.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/312#issuecomment-67759656 Thank you for your comment. If you think that some patch is acceptable, you can give one +1 for the patch. Then, the patch can be committed to a targeted branch. The Apache voting process is described at http://www.apache.org/foundation/voting.html .
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user ykrips commented on the pull request:

        https://github.com/apache/tajo/pull/312#issuecomment-67758348

        It is a good work to refactor Global Engine into small chunks of codes. I did not find any issues when running several queries on existing tajo cluster.

        Show
        githubbot ASF GitHub Bot added a comment - Github user ykrips commented on the pull request: https://github.com/apache/tajo/pull/312#issuecomment-67758348 It is a good work to refactor Global Engine into small chunks of codes. I did not find any issues when running several queries on existing tajo cluster.
        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/12688309/TAJO-1261.patch
        against master revision release-0.9.0-rc0-103-g3413107.

        +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 173 new Findbugs (version 2.0.3) warnings.

        -1 release audit. The applied patch generated 297 release audit warnings.

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

        Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/549//testReport/
        Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/549//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt
        Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/549//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html
        Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/549//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/12688309/TAJO-1261.patch against master revision release-0.9.0-rc0-103-g3413107. +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 173 new Findbugs (version 2.0.3) warnings. -1 release audit. The applied patch generated 297 release audit warnings. +1 core tests. The patch passed unit tests in tajo-core. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/549//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/549//artifact/incubator-tajo/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/549//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/549//console This message is automatically generated.
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user hyunsik opened a pull request:

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

        TAJO-1261: Separate query and ddl execution codes from GlobalEngine.

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

        $ git pull https://github.com/hyunsik/tajo TAJO-1261

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

        https://github.com/apache/tajo/pull/312.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 #312


        commit f7e2cd3c706de99d6a2a7d2d2ee69dba28fe3221
        Author: Hyunsik Choi <hyunsik@apache.org>
        Date: 2014-12-19T11:55:26Z

        TAJO-1261: Separate query and ddl execution codes from GlobalEngine.


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user hyunsik opened a pull request: https://github.com/apache/tajo/pull/312 TAJO-1261 : Separate query and ddl execution codes from GlobalEngine. You can merge this pull request into a Git repository by running: $ git pull https://github.com/hyunsik/tajo TAJO-1261 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/312.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 #312 commit f7e2cd3c706de99d6a2a7d2d2ee69dba28fe3221 Author: Hyunsik Choi <hyunsik@apache.org> Date: 2014-12-19T11:55:26Z TAJO-1261 : Separate query and ddl execution codes from GlobalEngine.

          People

          • Assignee:
            hyunsik Hyunsik Choi
            Reporter:
            hyunsik Hyunsik Choi
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development