-----------------------------------------------------------
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
> 
>

Reply via email to