Details

    • Sub-task
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 2.1.0-beta
    • ipc
    • Incompatible change, Reviewed

    Description

      IpcSerializationType is assumed to be protobuf for the forseeable future. Not to be confused with RpcKind which still supports different RpcEngines. Let's remove the dead code, which can be confusing to maintain.

      Attachments

        1. HADOOP-9630.patch
          4 kB
          Junping Du

        Activity

          junping_du Junping Du added a comment -

          Attach a patch to cleanup IpcSerializationType.

          junping_du Junping Du added a comment - Attach a patch to cleanup IpcSerializationType.
          hadoopqa Hadoop QA added a comment -

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

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

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

          +1 javadoc. The 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/2627//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2627//console

          This message is automatically generated.

          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12587176/HADOOP-9630.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The 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/2627//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2627//console This message is automatically generated.
          vicaya Luke Lu added a comment -

          The patch lgtm, +1. Will commit shortly.

          vicaya Luke Lu added a comment - The patch lgtm, +1. Will commit shortly.
          vicaya Luke Lu added a comment -

          Committed to trunk, branch-2 and 2.1-beta. Thanks Junping!

          vicaya Luke Lu added a comment - Committed to trunk, branch-2 and 2.1-beta. Thanks Junping!
          hudson Hudson added a comment -

          Integrated in Hadoop-trunk-Commit #3894 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3894/)
          HADOOP-9630. [RPC v9] Remove IpcSerializationType. (Junping Du via llu) (Revision 1491682)

          Result = SUCCESS
          llu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1491682
          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/ipc/Client.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
          hudson Hudson added a comment - Integrated in Hadoop-trunk-Commit #3894 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3894/ ) HADOOP-9630 . [RPC v9] Remove IpcSerializationType. (Junping Du via llu) (Revision 1491682) Result = SUCCESS llu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1491682 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/ipc/Client.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
          hudson Hudson added a comment -

          Integrated in Hadoop-Yarn-trunk #237 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/237/)
          HADOOP-9630. [RPC v9] Remove IpcSerializationType. (Junping Du via llu) (Revision 1491682)

          Result = SUCCESS
          llu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1491682
          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/ipc/Client.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
          hudson Hudson added a comment - Integrated in Hadoop-Yarn-trunk #237 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/237/ ) HADOOP-9630 . [RPC v9] Remove IpcSerializationType. (Junping Du via llu) (Revision 1491682) Result = SUCCESS llu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1491682 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/ipc/Client.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
          hudson Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1454 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1454/)
          HADOOP-9630. [RPC v9] Remove IpcSerializationType. (Junping Du via llu) (Revision 1491682)

          Result = SUCCESS
          llu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1491682
          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/ipc/Client.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
          hudson Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1454 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1454/ ) HADOOP-9630 . [RPC v9] Remove IpcSerializationType. (Junping Du via llu) (Revision 1491682) Result = SUCCESS llu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1491682 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/ipc/Client.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
          sanjay.radia Sanjay Radia added a comment -

          I missed this jira. Why do you want to disallow anyone adding a new serialization type in the future - its one byte you are saving.

          sanjay.radia Sanjay Radia added a comment - I missed this jira. Why do you want to disallow anyone adding a new serialization type in the future - its one byte you are saving.
          vicaya Luke Lu added a comment -

          For RPC v9, it's not likely have reasonable support of different serialization types for rpc headers and sasl exchange, we might as well stop pretending that it's possible. The main drive for this jira and the parent is to make the protocol consistent and simple so that alternative clients in different languages can be implemented and maintained with minimal cognitive overhead. It's doubtful that we'll ever need an alternative rpc header/sasl serialization type when the actual rpc serialization can be different via different rpc engines. If there is a real need for different serialization type for rpc header/sasl in the future, we can bring it back in future major versions of rpc. I sure hope that RPC v9 would be the last major RPC version, as people are banking on supporting backward-compatibility/rolling upgrade via the protobuf support.

          OTOH, I'm definitely interested in hearing about any actual plans to support different serialization types for rpc header/sasl exchanges and related the use cases.

          vicaya Luke Lu added a comment - For RPC v9, it's not likely have reasonable support of different serialization types for rpc headers and sasl exchange, we might as well stop pretending that it's possible. The main drive for this jira and the parent is to make the protocol consistent and simple so that alternative clients in different languages can be implemented and maintained with minimal cognitive overhead. It's doubtful that we'll ever need an alternative rpc header/sasl serialization type when the actual rpc serialization can be different via different rpc engines. If there is a real need for different serialization type for rpc header/sasl in the future, we can bring it back in future major versions of rpc. I sure hope that RPC v9 would be the last major RPC version, as people are banking on supporting backward-compatibility/rolling upgrade via the protobuf support. OTOH, I'm definitely interested in hearing about any actual plans to support different serialization types for rpc header/sasl exchanges and related the use cases.

          People

            junping_du Junping Du
            vicaya Luke Lu
            Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: