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

Remove QueryMaster client sharing in TajoMaster and TajoWorker

    Details

      Description

      QueryMaster client should remove after use. It can possible that connection refused in very large cluster because too many open connections

      1. TAJO-1584_2.patch
        20 kB
        Jinho Kim
      2. TAJO-1584_3.patch
        29 kB
        Jinho Kim
      3. TAJO-1584.patch
        17 kB
        Jinho Kim

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user jinossy opened a pull request:

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

          TAJO-1584: Remove QueryMaster client sharing in TajoMaster and TajoWorke...

          ...r

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

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

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

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


          commit 2c138e53e2e61617ef5797d5dbb88791b24de746
          Author: Jinho Kim <jhkim@apache.org>
          Date: 2015-04-29T10:43:30Z

          TAJO-1584: Remove QueryMaster client sharing in TajoMaster and TajoWorker


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user jinossy opened a pull request: https://github.com/apache/tajo/pull/559 TAJO-1584 : Remove QueryMaster client sharing in TajoMaster and TajoWorke... ...r You can merge this pull request into a Git repository by running: $ git pull https://github.com/jinossy/tajo TAJO-1584 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/559.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 #559 commit 2c138e53e2e61617ef5797d5dbb88791b24de746 Author: Jinho Kim <jhkim@apache.org> Date: 2015-04-29T10:43:30Z TAJO-1584 : Remove QueryMaster client sharing in TajoMaster and TajoWorker
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jinossy commented on the pull request:

          https://github.com/apache/tajo/pull/559#issuecomment-97386990

          I’ve add option to reuse configuration.
          QUERY_MASTER_RPC_CLIENT_REUSE("tajo.querymaster.rpc.client.reuse", false)

          Show
          githubbot ASF GitHub Bot added a comment - Github user jinossy commented on the pull request: https://github.com/apache/tajo/pull/559#issuecomment-97386990 I’ve add option to reuse configuration. QUERY_MASTER_RPC_CLIENT_REUSE("tajo.querymaster.rpc.client.reuse", false)
          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/12729135/TAJO-1584.patch
          against master revision release-0.9.0-rc0-279-g9540f16.

          +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 does not introduce any new Findbugs (version 2.0.3) 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-common tajo-core tajo-rpc/tajo-rpc-protobuf.

          Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/765//testReport/
          Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/765//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/12729135/TAJO-1584.patch against master revision release-0.9.0-rc0-279-g9540f16. +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 does not introduce any new Findbugs (version 2.0.3) 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-common tajo-core tajo-rpc/tajo-rpc-protobuf. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/765//testReport/ Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/765//console This message is automatically generated.
          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/12729154/TAJO-1584_2.patch
          against master revision release-0.9.0-rc0-279-g9540f16.

          +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 does not introduce any new Findbugs (version 2.0.3) 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-common tajo-core tajo-rpc/tajo-rpc-protobuf.

          Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/766//testReport/
          Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/766//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/12729154/TAJO-1584_2.patch against master revision release-0.9.0-rc0-279-g9540f16. +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 does not introduce any new Findbugs (version 2.0.3) 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-common tajo-core tajo-rpc/tajo-rpc-protobuf. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/766//testReport/ Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/766//console This message is automatically generated.
          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/559#discussion_r29414552

          — Diff: tajo-core/src/main/java/org/apache/tajo/master/QueryInProgress.java —
          @@ -156,8 +161,15 @@ public boolean startQueryMaster() {
          private void connectQueryMaster() throws Exception {
          InetSocketAddress addr = NetUtils.createSocketAddr(queryInfo.getQueryMasterHost(), queryInfo.getQueryMasterPort());
          LOG.info("Connect to QueryMaster:" + addr);

          • queryMasterRpc =
          • RpcClientManager.getInstance().getClient(addr, QueryMasterProtocol.class, true);
            +
            + /* A large scale cluster can be connection refused, if it reuse */
              • End diff –

          It would be better if it should be changed to 'Connection can be refused in large scale clusters if 'reuse' option is TRUE'.

          In addition, this description should be also described in ```QUERY_MASTER_RPC_CLIENT_REUSE``` in TajoConf and user guide.

          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/559#discussion_r29414552 — Diff: tajo-core/src/main/java/org/apache/tajo/master/QueryInProgress.java — @@ -156,8 +161,15 @@ public boolean startQueryMaster() { private void connectQueryMaster() throws Exception { InetSocketAddress addr = NetUtils.createSocketAddr(queryInfo.getQueryMasterHost(), queryInfo.getQueryMasterPort()); LOG.info("Connect to QueryMaster:" + addr); queryMasterRpc = RpcClientManager.getInstance().getClient(addr, QueryMasterProtocol.class, true); + + /* A large scale cluster can be connection refused, if it reuse */ End diff – It would be better if it should be changed to 'Connection can be refused in large scale clusters if 'reuse' option is TRUE'. In addition, this description should be also described in ```QUERY_MASTER_RPC_CLIENT_REUSE``` in TajoConf and user guide.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hyunsik commented on the pull request:

          https://github.com/apache/tajo/pull/559#issuecomment-97722324

          In this patch, the connection reuse is changed to be optional. In my opinion, it would be better if we completely remove the reuse feature. Actually, I'm not sure if anyone want 'reuse feature'.

          Others look good to me.

          Show
          githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/559#issuecomment-97722324 In this patch, the connection reuse is changed to be optional. In my opinion, it would be better if we completely remove the reuse feature. Actually, I'm not sure if anyone want 'reuse feature'. Others look good to me.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jinossy commented on the pull request:

          https://github.com/apache/tajo/pull/559#issuecomment-97725375

          I agree with you. I will remove the 'reuse feature'

          Show
          githubbot ASF GitHub Bot added a comment - Github user jinossy commented on the pull request: https://github.com/apache/tajo/pull/559#issuecomment-97725375 I agree with you. I will remove the 'reuse feature'
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jinossy commented on the pull request:

          https://github.com/apache/tajo/pull/559#issuecomment-99299271

          @hyunsik
          Could you review again?

          Show
          githubbot ASF GitHub Bot added a comment - Github user jinossy commented on the pull request: https://github.com/apache/tajo/pull/559#issuecomment-99299271 @hyunsik Could you review again?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hyunsik commented on the pull request:

          https://github.com/apache/tajo/pull/559#issuecomment-99389833

          +1 ship it!!

          Show
          githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/559#issuecomment-99389833 +1 ship it!!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

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

          committed it
          Thanks

          Show
          jhkim Jinho Kim added a comment - committed it Thanks
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Tajo-master-build #694 (See https://builds.apache.org/job/Tajo-master-build/694/)
          TAJO-1584: Remove QueryMaster client sharing in TajoMaster and TajoWorker. (jhkim: rev 04167bdc3bb04b53c5a245a9c18b6426ade82a26)

          • CHANGES
          • tajo-core/src/main/java/org/apache/tajo/worker/ExecutionBlockContext.java
          • tajo-core/src/main/java/org/apache/tajo/master/QueryInProgress.java
          • tajo-core/src/main/java/org/apache/tajo/worker/TajoWorkerManagerService.java
          • tajo-core/src/main/java/org/apache/tajo/worker/TajoWorker.java
          • tajo-rpc/tajo-rpc-protobuf/src/main/java/org/apache/tajo/rpc/RpcClientManager.java
          • tajo-core/src/main/java/org/apache/tajo/worker/TaskRunner.java
          • tajo-core/src/main/java/org/apache/tajo/querymaster/QueryMasterManagerService.java
          • tajo-core/src/main/proto/QueryMasterProtocol.proto
          • tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java
          • tajo-rpc/tajo-rpc-protobuf/src/main/java/org/apache/tajo/rpc/NettyClientBase.java
          • tajo-core/src/main/java/org/apache/tajo/worker/Task.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #694 (See https://builds.apache.org/job/Tajo-master-build/694/ ) TAJO-1584 : Remove QueryMaster client sharing in TajoMaster and TajoWorker. (jhkim: rev 04167bdc3bb04b53c5a245a9c18b6426ade82a26) CHANGES tajo-core/src/main/java/org/apache/tajo/worker/ExecutionBlockContext.java tajo-core/src/main/java/org/apache/tajo/master/QueryInProgress.java tajo-core/src/main/java/org/apache/tajo/worker/TajoWorkerManagerService.java tajo-core/src/main/java/org/apache/tajo/worker/TajoWorker.java tajo-rpc/tajo-rpc-protobuf/src/main/java/org/apache/tajo/rpc/RpcClientManager.java tajo-core/src/main/java/org/apache/tajo/worker/TaskRunner.java tajo-core/src/main/java/org/apache/tajo/querymaster/QueryMasterManagerService.java tajo-core/src/main/proto/QueryMasterProtocol.proto tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java tajo-rpc/tajo-rpc-protobuf/src/main/java/org/apache/tajo/rpc/NettyClientBase.java tajo-core/src/main/java/org/apache/tajo/worker/Task.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Tajo-master-CODEGEN-build #333 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/333/)
          TAJO-1584: Remove QueryMaster client sharing in TajoMaster and TajoWorker. (jhkim: rev 04167bdc3bb04b53c5a245a9c18b6426ade82a26)

          • tajo-core/src/main/java/org/apache/tajo/worker/TajoWorker.java
          • tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java
          • tajo-core/src/main/proto/QueryMasterProtocol.proto
          • tajo-core/src/main/java/org/apache/tajo/worker/Task.java
          • tajo-core/src/main/java/org/apache/tajo/querymaster/QueryMasterManagerService.java
          • tajo-rpc/tajo-rpc-protobuf/src/main/java/org/apache/tajo/rpc/RpcClientManager.java
          • tajo-core/src/main/java/org/apache/tajo/worker/TaskRunner.java
          • tajo-rpc/tajo-rpc-protobuf/src/main/java/org/apache/tajo/rpc/NettyClientBase.java
          • tajo-core/src/main/java/org/apache/tajo/worker/TajoWorkerManagerService.java
          • tajo-core/src/main/java/org/apache/tajo/master/QueryInProgress.java
          • tajo-core/src/main/java/org/apache/tajo/worker/ExecutionBlockContext.java
          • CHANGES
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #333 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/333/ ) TAJO-1584 : Remove QueryMaster client sharing in TajoMaster and TajoWorker. (jhkim: rev 04167bdc3bb04b53c5a245a9c18b6426ade82a26) tajo-core/src/main/java/org/apache/tajo/worker/TajoWorker.java tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java tajo-core/src/main/proto/QueryMasterProtocol.proto tajo-core/src/main/java/org/apache/tajo/worker/Task.java tajo-core/src/main/java/org/apache/tajo/querymaster/QueryMasterManagerService.java tajo-rpc/tajo-rpc-protobuf/src/main/java/org/apache/tajo/rpc/RpcClientManager.java tajo-core/src/main/java/org/apache/tajo/worker/TaskRunner.java tajo-rpc/tajo-rpc-protobuf/src/main/java/org/apache/tajo/rpc/NettyClientBase.java tajo-core/src/main/java/org/apache/tajo/worker/TajoWorkerManagerService.java tajo-core/src/main/java/org/apache/tajo/master/QueryInProgress.java tajo-core/src/main/java/org/apache/tajo/worker/ExecutionBlockContext.java CHANGES

            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