You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "Brandon Li (JIRA)" <ji...@apache.org> on 2012/08/27 22:13:07 UTC

[jira] [Created] (HADOOP-8736) Create a Builder to make an RPC server

Brandon Li created HADOOP-8736:
----------------------------------

             Summary: Create a Builder to make an RPC server
                 Key: HADOOP-8736
                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
             Project: Hadoop Common
          Issue Type: Improvement
          Components: ipc
    Affects Versions: 3.0.0
            Reporter: Brandon Li
            Assignee: Brandon Li


There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13443683#comment-13443683 ] 

Brandon Li commented on HADOOP-8736:
------------------------------------

Uploaded a new patch to address the above comments.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Brandon Li updated HADOOP-8736:
-------------------------------

    Attachment: HADOOP-8736.patch
    
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13442819#comment-13442819 ] 

Hadoop QA commented on HADOOP-8736:
-----------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12542693/HADOOP-8736.patch
  against trunk revision .

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

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

    -1 javac.  The applied patch generated 2069 javac compiler warnings (more than the trunk's current 2059 warnings).

    +1 javadoc.  The javadoc tool did not generate any warning messages.

    +1 eclipse:eclipse.  The patch built with eclipse:eclipse.

    +1 findbugs.  The patch does not introduce any 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 hadoop-common-project/hadoop-common.

    +1 contrib tests.  The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1370//testReport/
Javac warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/1370//artifact/trunk/patchprocess/diffJavacWarnings.txt
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1370//console

This message is automatically generated.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Add Builder for building an RPC server

Posted by "Hudson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13461151#comment-13461151 ] 

Hudson commented on HADOOP-8736:
--------------------------------

Integrated in Hadoop-Mapreduce-trunk #1204 (See [https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1204/])
    Moving HADOOP-8736 to release 2.0.3 section (Revision 1388578)

     Result = FAILURE
suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388578
Files : 
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt

                
> Add Builder for building an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>             Fix For: 3.0.0, 2.0.3-alpha
>
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13446184#comment-13446184 ] 

Brandon Li commented on HADOOP-8736:
------------------------------------

Updated the patch. Thanks!
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Add Builder for building an RPC server

Posted by "Hudson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13446707#comment-13446707 ] 

Hudson commented on HADOOP-8736:
--------------------------------

Integrated in Hadoop-Mapreduce-trunk #1183 (See [https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1183/])
    HADOOP-8736. Add Builder for building RPC server. Contributed by Brandon Li (Revision 1379652)

     Result = SUCCESS
suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1379652
Files : 
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ZKFCRpcServer.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/MiniRPCBenchmark.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/RPCCallBenchmark.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestMultipleProtocolServer.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestProtoBufRpc.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPCCompatibility.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestSaslRPC.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestDoAsEffectiveUser.java

                
> Add Builder for building an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>             Fix For: 3.0.0
>
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13443730#comment-13443730 ] 

Hadoop QA commented on HADOOP-8736:
-----------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12542869/HADOOP-8736.patch
  against trunk revision .

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

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

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

    +1 javadoc.  The javadoc tool did not generate any warning messages.

    +1 eclipse:eclipse.  The patch built with eclipse:eclipse.

    +1 findbugs.  The patch does not introduce any 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 failed these unit tests in hadoop-common-project/hadoop-common:

                  org.apache.hadoop.ha.TestZKFailoverController

    +1 contrib tests.  The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1376//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1376//console

This message is automatically generated.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Add Builder for building an RPC server

Posted by "Hudson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13460655#comment-13460655 ] 

Hudson commented on HADOOP-8736:
--------------------------------

Integrated in Hadoop-Common-trunk-Commit #2753 (See [https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2753/])
    Moving HADOOP-8736 to release 2.0.3 section (Revision 1388578)

     Result = SUCCESS
suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388578
Files : 
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt

                
> Add Builder for building an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>             Fix For: 3.0.0, 2.0.3-alpha
>
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Brandon Li updated HADOOP-8736:
-------------------------------

    Attachment: HADOOP-8736.patch
    
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13444515#comment-13444515 ] 

Brandon Li commented on HADOOP-8736:
------------------------------------

{quote}build() method, please follow the coding guidelines and have {} after if.{quote}
done
{quote}Throwing HadoopIllegalArgumentException is fine. But if you are doing that for two of the parameters, I suggest doing the same for handlerCount and conf parameter as well.{quote}
Done for conf, handlerCount has a default as "1".
{quote} In javadoc for build() method please add @throws and say if mandator fields are not set, the build method will throw HadoopIllegalArgumentException.{quote}
Done.
{quote} Please add a testcase where you create RPC server without mandatory fields and ensure exceptions are thrown. We could perhaps add this to TestIPC.{quote}
Done in TestRPC.java.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Add Builder for building an RPC server

Posted by "Hudson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13446679#comment-13446679 ] 

Hudson commented on HADOOP-8736:
--------------------------------

Integrated in Hadoop-Hdfs-trunk #1152 (See [https://builds.apache.org/job/Hadoop-Hdfs-trunk/1152/])
    HADOOP-8736. Add Builder for building RPC server. Contributed by Brandon Li (Revision 1379652)

     Result = SUCCESS
suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1379652
Files : 
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ZKFCRpcServer.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/MiniRPCBenchmark.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/RPCCallBenchmark.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestMultipleProtocolServer.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestProtoBufRpc.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPCCompatibility.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestSaslRPC.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestDoAsEffectiveUser.java

                
> Add Builder for building an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>             Fix For: 3.0.0
>
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (HADOOP-8736) Add Builder to for building an RPC server

Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Suresh Srinivas updated HADOOP-8736:
------------------------------------

    Summary: Add Builder to for building an RPC server  (was: Create a Builder to make an RPC server)
    
> Add Builder to for building an RPC server
> -----------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Comment Edited] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13443244#comment-13443244 ] 

Suresh Srinivas edited comment on HADOOP-8736 at 8/29/12 3:56 AM:
------------------------------------------------------------------

bq. but I think it should be "setPort".
Correct.

bq. Or perhaps those required fields should be set as part of the Builder constructor, while the other fields are set via the builder set*() methods?
Actually I prefer set* methods instead of Builder constructor. That way adding another constructor for another required field is not required.

Thinking more about this, I think the way code is done is probably fine. For required fields, setting to null that results in NullPointerException to indicate programming error is fine. Not sure if there are any other fields which will not cause runtime exception where the default value cannot be valid.
                
      was (Author: sureshms):
    bq. but I think it should be "setPort".
Correct.

bq. Or perhaps those required fields should be set as part of the Builder constructor, while the other fields are set via the builder set*() methods?
Actually I prefer set* methods instead of Builder constructor. That way adding another constructor for another required field is not required.

Thinking more about this, I think the way code is done is probably fine. For required fields, setting to null that results in NullPointerException to indicate programming error is fine. Not sure if there are any other fields which will not cause runtime exception and some invalid default is considered.
                  
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Brandon Li updated HADOOP-8736:
-------------------------------

    Status: Patch Available  (was: Open)
    
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13442958#comment-13442958 ] 

Brandon Li commented on HADOOP-8736:
------------------------------------

The new javac warnings are due to the new deprecated getServer() method in the patch.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Add Builder for building an RPC server

Posted by "Eli Collins (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13457994#comment-13457994 ] 

Eli Collins commented on HADOOP-8736:
-------------------------------------

Consider merging to branch-2? Seems like a straight-forward change and will help with merge conflicts.
                
> Add Builder for building an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>             Fix For: 3.0.0
>
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13443244#comment-13443244 ] 

Suresh Srinivas commented on HADOOP-8736:
-----------------------------------------

bq. but I think it should be "setPort".
Correct.

bq. Or perhaps those required fields should be set as part of the Builder constructor, while the other fields are set via the builder set*() methods?
Actually I prefer set* methods instead of Builder constructor. That way adding another constructor for another required field is not required.

Thinking more about this, I think the way code is done is probably fine. For required fields, setting to null that results in NullPointerException to indicate programming error is fine. Not sure if there are any other fields which will not cause runtime exception and some invalid default is considered.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Aaron T. Myers (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13444287#comment-13444287 ] 

Aaron T. Myers commented on HADOOP-8736:
----------------------------------------

bq. Firstly, the "truly required fields" are just the truly required for now, and it's hard to predict future.

Sure, but if/when that future comes, what will have to happen with each of these approaches? In the constructor approach, you'll change the constructor signature and then things won't compile until you've fixed all of the call sites. That is good. In the multi-method builder approach, everything will compile, but you'll have to run all of the tests to find the call sites that you missed when adding a new builder method call, and also will have to hope that the tests in fact do cover all of the call sites. That is bad.

bq. Secondly, even we have a constructor with all the current required fields, the developer can still pass null pointers by mistake.

Of course that's always a possibility, but that seems less likely than a developer forgetting to call a builder method that is in fact required. That's also a possibility with the multi-method builder approach as well - the developer might pass null values by mistake.

But like I said, you can go with whatever you prefer. You haven't convinced me that this is the right way to go, but I'm not going to stop you from doing it.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Brandon Li updated HADOOP-8736:
-------------------------------

    Attachment: HADOOP-8736.patch
    
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (HADOOP-8736) Add Builder for building an RPC server

Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Suresh Srinivas updated HADOOP-8736:
------------------------------------

    Summary: Add Builder for building an RPC server  (was: Add Builder to for building an RPC server)
    
> Add Builder for building an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13443273#comment-13443273 ] 

Brandon Li commented on HADOOP-8736:
------------------------------------

In the patch, I tried to keep the Builder constructor simple(taking only Configuration as input). I think it can give the Builder more flexibility. For example, suppose the constructor takes 3 arguments, conf, protocol and instance. Suppose later on we realized more fields can not be null, we have to either modify the constructor or create a few new constructor, which make it like getServer() again. 

If we want to avoid NullPointerException, we could do a sanity check in the last step build() and throw an exception. The sanity check can be more comprehensive as more needs added later on.

                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13446255#comment-13446255 ] 

Hadoop QA commented on HADOOP-8736:
-----------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12543312/HADOOP-8736.patch
  against trunk revision .

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

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

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

    +1 javadoc.  The javadoc tool did not generate any warning messages.

    +1 eclipse:eclipse.  The patch built with eclipse:eclipse.

    +1 findbugs.  The patch does not introduce any 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 failed these unit tests in hadoop-common-project/hadoop-common:

                  org.apache.hadoop.ha.TestZKFailoverController

    +1 contrib tests.  The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1392//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1392//console

This message is automatically generated.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13444531#comment-13444531 ] 

Hadoop QA commented on HADOOP-8736:
-----------------------------------

+1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12543003/HADOOP-8736.patch
  against trunk revision .

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

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

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

    +1 javadoc.  The javadoc tool did not generate any warning messages.

    +1 eclipse:eclipse.  The patch built with eclipse:eclipse.

    +1 findbugs.  The patch does not introduce any 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 hadoop-common-project/hadoop-common.

    +1 contrib tests.  The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1385//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1385//console

This message is automatically generated.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13446259#comment-13446259 ] 

Brandon Li commented on HADOOP-8736:
------------------------------------

The test failure is not introduced by this patch.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Aaron T. Myers (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13443285#comment-13443285 ] 

Aaron T. Myers commented on HADOOP-8736:
----------------------------------------

I see your points, but I'd still prefer to have the truly required fields in the constructor. That way, if we later add another required field, developers' IDEs will pick this up or it will be seen at compile time, instead of waiting for a RTE like NPE or some specific exception we put in the build() method. For what are truly required fields, it seems strictly better to me to catch this at compile time instead of later.

I don't feel super strongly about this. Feel free to roll with whatever you'd prefer.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Brandon Li updated HADOOP-8736:
-------------------------------

    Attachment: HADOOP-8736.patch
    
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13443069#comment-13443069 ] 

Suresh Srinivas commented on HADOOP-8736:
-----------------------------------------

Thanks Brandon for doing this. This cleanup improves the readability of the code.

I have not reviewed the patch completely. Here are my early comments:
# Can you add javadoc to the newly added class {{Builder}}
# Instead of deprecating getServer() methods, why not get rid of those methods altogether? You could also leave those methods for now and get rid of them in another jira.
# Typo: method name "setProt" should be "setProto"
# Some of the values while building protocol engine, such as protocol, instance, etc. cannot have default values. It would be good to either add check for that in Builder or ensure it is is checked in {{getProtocolEngine()}} implementation.

                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13442737#comment-13442737 ] 

Hadoop QA commented on HADOOP-8736:
-----------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12542664/HADOOP-8736.patch
  against trunk revision .

    -1 patch.  The patch command could not apply the patch.

Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1369//console

This message is automatically generated.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13446160#comment-13446160 ] 

Suresh Srinivas commented on HADOOP-8736:
-----------------------------------------

Minor comments:
# Please add information which of the fields are mandatory
# Javadoc for build() method should use java convention @throws and explain in that HadoopIllegalArgumentException is thrown when mandatory fields are not set.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Add Builder for building an RPC server

Posted by "Hudson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13460677#comment-13460677 ] 

Hudson commented on HADOOP-8736:
--------------------------------

Integrated in Hadoop-Mapreduce-trunk-Commit #2775 (See [https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2775/])
    Moving HADOOP-8736 to release 2.0.3 section (Revision 1388578)

     Result = FAILURE
suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388578
Files : 
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt

                
> Add Builder for building an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>             Fix For: 3.0.0, 2.0.3-alpha
>
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13444406#comment-13444406 ] 

Suresh Srinivas commented on HADOOP-8736:
-----------------------------------------

bq. But like I said, you can go with whatever you prefer. You haven't convinced me that this is the right way to go, but I'm not going to stop you from doing it.
I am fine with the current approach being taken. If there are issue due to this approach, because of frequent changes in Server, we could always revisit this.

Here are comments for the patch (mostly nits):
# build() method, please follow the coding guidelines and have {} after if.
# Throwing HadoopIllegalArgumentException is fine. But if you are doing that for two of the parameters, I suggest doing the same for handlerCount and conf parameter as well.
# In javadoc for build() method please add @throws and say if mandator fields are not set, the build method will throw HadoopIllegalArgumentException.
# Please add a testcase where you create RPC server without mandatory fields and ensure exceptions are thrown. We could perhaps add this to TestIPC.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Add Builder for building an RPC server

Posted by "Hudson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13446488#comment-13446488 ] 

Hudson commented on HADOOP-8736:
--------------------------------

Integrated in Hadoop-Mapreduce-trunk-Commit #2694 (See [https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2694/])
    HADOOP-8736. Add Builder for building RPC server. Contributed by Brandon Li (Revision 1379652)

     Result = FAILURE
suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1379652
Files : 
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ZKFCRpcServer.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/MiniRPCBenchmark.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/RPCCallBenchmark.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestMultipleProtocolServer.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestProtoBufRpc.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPCCompatibility.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestSaslRPC.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestDoAsEffectiveUser.java

                
> Add Builder for building an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>             Fix For: 3.0.0
>
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (HADOOP-8736) Add Builder for building an RPC server

Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Suresh Srinivas updated HADOOP-8736:
------------------------------------

    Fix Version/s: 2.0.3-alpha
    
> Add Builder for building an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>             Fix For: 3.0.0, 2.0.3-alpha
>
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Brandon Li updated HADOOP-8736:
-------------------------------

    Attachment: HADOOP-8736.patch
    
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (HADOOP-8736) Add Builder for building an RPC server

Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Suresh Srinivas updated HADOOP-8736:
------------------------------------

       Resolution: Fixed
    Fix Version/s: 3.0.0
     Hadoop Flags: Reviewed
           Status: Resolved  (was: Patch Available)
    
> Add Builder for building an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>             Fix For: 3.0.0
>
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Add Builder for building an RPC server

Posted by "Hudson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13461143#comment-13461143 ] 

Hudson commented on HADOOP-8736:
--------------------------------

Integrated in Hadoop-Hdfs-trunk #1173 (See [https://builds.apache.org/job/Hadoop-Hdfs-trunk/1173/])
    Moving HADOOP-8736 to release 2.0.3 section (Revision 1388578)

     Result = SUCCESS
suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388578
Files : 
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt

                
> Add Builder for building an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>             Fix For: 3.0.0, 2.0.3-alpha
>
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Add Builder for building an RPC server

Posted by "Hudson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13446470#comment-13446470 ] 

Hudson commented on HADOOP-8736:
--------------------------------

Integrated in Hadoop-Common-trunk-Commit #2669 (See [https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2669/])
    HADOOP-8736. Add Builder for building RPC server. Contributed by Brandon Li (Revision 1379652)

     Result = SUCCESS
suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1379652
Files : 
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ZKFCRpcServer.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/MiniRPCBenchmark.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/RPCCallBenchmark.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestMultipleProtocolServer.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestProtoBufRpc.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPCCompatibility.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestSaslRPC.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestDoAsEffectiveUser.java

                
> Add Builder for building an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>             Fix For: 3.0.0
>
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Add Builder for building an RPC server

Posted by "Hudson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13446467#comment-13446467 ] 

Hudson commented on HADOOP-8736:
--------------------------------

Integrated in Hadoop-Hdfs-trunk-Commit #2732 (See [https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2732/])
    HADOOP-8736. Add Builder for building RPC server. Contributed by Brandon Li (Revision 1379652)

     Result = SUCCESS
suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1379652
Files : 
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ZKFCRpcServer.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/MiniRPCBenchmark.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/RPCCallBenchmark.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestMultipleProtocolServer.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestProtoBufRpc.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPCCompatibility.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestSaslRPC.java
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestDoAsEffectiveUser.java

                
> Add Builder for building an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>             Fix For: 3.0.0
>
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Add Builder for building an RPC server

Posted by "Suresh Srinivas (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13460637#comment-13460637 ] 

Suresh Srinivas commented on HADOOP-8736:
-----------------------------------------

Merged this change to branch-2.
                
> Add Builder for building an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>             Fix For: 3.0.0, 2.0.3-alpha
>
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13444389#comment-13444389 ] 

Brandon Li commented on HADOOP-8736:
------------------------------------

Aaron, I appreciate your review and comments. Discussion helps. :-)
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13443605#comment-13443605 ] 

Brandon Li commented on HADOOP-8736:
------------------------------------

{quote} That way, if we later add another required field, developers' IDEs will pick this up or it will be seen at compile time, instead of waiting for a RTE like NPE or some specific exception we put in the build() method.{quote}
I agree that bigger constructor has certain merits. However, there are two problems. Firstly, the "truly required fields" are just the truly required for now, and it's hard to predict future. Secondly, even we have a constructor with all the current required fields, the developer can still pass null pointers by mistake. 

Doing param checking and throwing exceptions in last step doesn't seem too strange. Another example doing this is in google library https://developers.google.com/protocol-buffers/docs/reference/java/com/google/protobuf/MessageLite.Builder#build%28%29

                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13443788#comment-13443788 ] 

Brandon Li commented on HADOOP-8736:
------------------------------------

The test failure is not introduced by this patch, and it passed in my local test.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Brandon Li (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13442805#comment-13442805 ] 

Brandon Li commented on HADOOP-8736:
------------------------------------

Uploaded a new patch which is rebased with the HEAD of trunk.
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Add Builder for building an RPC server

Posted by "Hudson (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13460653#comment-13460653 ] 

Hudson commented on HADOOP-8736:
--------------------------------

Integrated in Hadoop-Hdfs-trunk-Commit #2816 (See [https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2816/])
    Moving HADOOP-8736 to release 2.0.3 section (Revision 1388578)

     Result = SUCCESS
suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1388578
Files : 
* /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt

                
> Add Builder for building an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>             Fix For: 3.0.0, 2.0.3-alpha
>
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HADOOP-8736) Create a Builder to make an RPC server

Posted by "Aaron T. Myers (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-8736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13443226#comment-13443226 ] 

Aaron T. Myers commented on HADOOP-8736:
----------------------------------------

bq. Typo: method name "setProt" should be "setProto"

Agree it's a typo, but I think it should be "setPort".

bq. Some of the values while building protocol engine, such as protocol, instance, etc. cannot have default values. It would be good to either add check for that in Builder or ensure it is is checked in getProtocolEngine() implementation.

Or perhaps those required fields should be set as part of the Builder constructor, while the other fields are set via the builder set*() methods?
                
> Create a Builder to make an RPC server
> --------------------------------------
>
>                 Key: HADOOP-8736
>                 URL: https://issues.apache.org/jira/browse/HADOOP-8736
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: ipc
>    Affects Versions: 3.0.0
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HADOOP-8736.patch, HADOOP-8736.patch
>
>
> There are quite a few variants of getServer() method to create an RPC server. Create a builder class to abstract the building steps and avoid more getServer() variants in the future.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira