You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by Dae Myung Kang <ch...@gmail.com> on 2014/03/06 17:32:16 UTC

Review Request 18847: Extract some network setting value to conf

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18847/
-----------------------------------------------------------

Review request for Tajo.


Bugs: https://issues.apache.org/jira/browse/TAJO-623
    https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/TAJO-623


Repository: tajo


Description
-------

Currently, to chnage network buffer size or tcp_nodelay, or keepalive options, we should change source code.

```c
<property>
<name>tajo.network.receivebuffersize</name>
<value>10485760</value>
</property>
```

NETWORK_REUSEADDRESS("tajo.network.reuseaddress", true)
NETWORK_TCP_NODELAY("tajo.network.tcpnodelay", true)
NETWORK_KEEP_ALIVE("tajo.network.keepalive", true)
NETWORK_CONNECT_TIMEOUT_MILLIS("tajo.network.connecttimeoutmillis", 10000)
NETWORK_CONNECT_RESPONSE_TIMEOUT_MILLIS("tajo.network.connectresponsetimeoutmillis", 10000)
NETWORK_RECEIVE_BUFFERSIZE("tajo.network.receivebuffersize", 1048576 * 10)


Diffs
-----

  tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java 17eaa9a 
  tajo-common/src/test/java/org/apache/tajo/conf/TestTajoConf.java PRE-CREATION 
  tajo-rpc/src/main/java/org/apache/tajo/rpc/NettyClientBase.java 8373c37 
  tajo-rpc/src/main/java/org/apache/tajo/rpc/NettyServerBase.java 8f98d3a 
  tajo-rpc/src/main/java/org/apache/tajo/rpc/ProtoPipelineFactory.java b2a2004 

Diff: https://reviews.apache.org/r/18847/diff/


Testing
-------

Passed: mvn clean install
Passed: add unittest to check tajoconf


Thanks,

Dae Myung Kang


Re: Review Request 18847: Extract some network setting value to conf

Posted by Dae Myung Kang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18847/
-----------------------------------------------------------

(Updated March 19, 2014, 5:24 p.m.)


Review request for Tajo.


Changes
-------

I applied reviews


Bugs: https://issues.apache.org/jira/browse/TAJO-623
    https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/TAJO-623


Repository: tajo


Description
-------

Currently, to chnage network buffer size or tcp_nodelay, or keepalive options, we should change source code.

```c
<property>
<name>tajo.network.receivebuffersize</name>
<value>10485760</value>
</property>
```

NETWORK_REUSEADDRESS("tajo.network.reuseaddress", true)
NETWORK_TCP_NODELAY("tajo.network.tcpnodelay", true)
NETWORK_KEEP_ALIVE("tajo.network.keepalive", true)
NETWORK_CONNECT_TIMEOUT_MILLIS("tajo.network.connecttimeoutmillis", 10000)
NETWORK_CONNECT_RESPONSE_TIMEOUT_MILLIS("tajo.network.connectresponsetimeoutmillis", 10000)
NETWORK_RECEIVE_BUFFERSIZE("tajo.network.receivebuffersize", 1048576 * 10)


Diffs (updated)
-----

  tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java 621b475 
  tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java 3ca17a0 
  tajo-common/src/test/java/org/apache/tajo/conf/TestTajoConf.java PRE-CREATION 
  tajo-common/src/test/resources/tajo-rpc.xml PRE-CREATION 
  tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterClientService.java dc70f23 
  tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/TajoMasterService.java 5e9f729 
  tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMasterManagerService.java e3e0260 
  tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/rm/TajoResourceTracker.java 1bcf38b 
  tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoWorkerClientService.java a73623f 
  tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoWorkerManagerService.java 392a7cf 
  tajo-rpc/src/main/java/org/apache/tajo/rpc/AsyncRpcClient.java c84d6b6 
  tajo-rpc/src/main/java/org/apache/tajo/rpc/AsyncRpcServer.java b7e3cb6 
  tajo-rpc/src/main/java/org/apache/tajo/rpc/BlockingRpcClient.java 604ed52 
  tajo-rpc/src/main/java/org/apache/tajo/rpc/BlockingRpcServer.java 067d824 
  tajo-rpc/src/main/java/org/apache/tajo/rpc/NettyClientBase.java 8373c37 
  tajo-rpc/src/main/java/org/apache/tajo/rpc/ProtoPipelineFactory.java b2a2004 
  tajo-rpc/src/main/java/org/apache/tajo/rpc/RpcConnectionPool.java 777281f 
  tajo-rpc/src/test/java/org/apache/tajo/rpc/TestAsyncRpc.java 97bc28d 
  tajo-rpc/src/test/java/org/apache/tajo/rpc/TestBlockingRpc.java dedd96e 

Diff: https://reviews.apache.org/r/18847/diff/


Testing
-------

Passed: mvn clean install
Passed: add unittest to check tajoconf


Thanks,

Dae Myung Kang


Re: Review Request 18847: Extract some network setting value to conf

Posted by Hyunsik Choi <hy...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18847/#review36519
-----------------------------------------------------------


Thank you for your contribution. I added some comments.


tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java
<https://reviews.apache.org/r/18847/#comment67511>

    These settings are only used in rpc module. So, I think that the prefix 'tajo.rpc' is proper than 'tajo.network'. Actually, 'tajo.network' appears to be too general.
    
    Could you rename the prefix?



tajo-rpc/src/main/java/org/apache/tajo/rpc/NettyClientBase.java
<https://reviews.apache.org/r/18847/#comment67512>

    Such a TajoConf creation approach will cause limited use of configuration because we cannot pass some configs specified after startup to NettyClientBase.
    
    I'd like to suggest using constructors to take a TajoConf instance.



tajo-rpc/src/main/java/org/apache/tajo/rpc/NettyServerBase.java
<https://reviews.apache.org/r/18847/#comment67514>

    It's the same to above comment.



tajo-rpc/src/main/java/org/apache/tajo/rpc/ProtoPipelineFactory.java
<https://reviews.apache.org/r/18847/#comment67513>

    As I mentioned above, I'd suggest using constructors to take a TajoConf instance.


- Hyunsik Choi


On March 7, 2014, 1:32 a.m., Dae Myung Kang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18847/
> -----------------------------------------------------------
> 
> (Updated March 7, 2014, 1:32 a.m.)
> 
> 
> Review request for Tajo.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/TAJO-623
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/TAJO-623
> 
> 
> Repository: tajo
> 
> 
> Description
> -------
> 
> Currently, to chnage network buffer size or tcp_nodelay, or keepalive options, we should change source code.
> 
> ```c
> <property>
> <name>tajo.network.receivebuffersize</name>
> <value>10485760</value>
> </property>
> ```
> 
> NETWORK_REUSEADDRESS("tajo.network.reuseaddress", true)
> NETWORK_TCP_NODELAY("tajo.network.tcpnodelay", true)
> NETWORK_KEEP_ALIVE("tajo.network.keepalive", true)
> NETWORK_CONNECT_TIMEOUT_MILLIS("tajo.network.connecttimeoutmillis", 10000)
> NETWORK_CONNECT_RESPONSE_TIMEOUT_MILLIS("tajo.network.connectresponsetimeoutmillis", 10000)
> NETWORK_RECEIVE_BUFFERSIZE("tajo.network.receivebuffersize", 1048576 * 10)
> 
> 
> Diffs
> -----
> 
>   tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java 17eaa9a 
>   tajo-common/src/test/java/org/apache/tajo/conf/TestTajoConf.java PRE-CREATION 
>   tajo-rpc/src/main/java/org/apache/tajo/rpc/NettyClientBase.java 8373c37 
>   tajo-rpc/src/main/java/org/apache/tajo/rpc/NettyServerBase.java 8f98d3a 
>   tajo-rpc/src/main/java/org/apache/tajo/rpc/ProtoPipelineFactory.java b2a2004 
> 
> Diff: https://reviews.apache.org/r/18847/diff/
> 
> 
> Testing
> -------
> 
> Passed: mvn clean install
> Passed: add unittest to check tajoconf
> 
> 
> Thanks,
> 
> Dae Myung Kang
> 
>