Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.0-alpha
    • Component/s: ipc
    • Labels:
      None
    1. rpcPayload1.patch
      15 kB
      Sanjay Radia
    2. rpcPayload2.patch
      21 kB
      Sanjay Radia
    3. rpcPayload3.patch
      23 kB
      Sanjay Radia
    4. rpcPayload5.patch
      25 kB
      Sanjay Radia
    5. RpcPayLoadHeader.java
      3 kB
      Sanjay Radia

      Activity

      Hide
      Sanjay Radia added a comment -

      Early draft of the payload header.
      Note it is neutral to specific serialization technologies such as PB, Avro, Thrift

      Show
      Sanjay Radia added a comment - Early draft of the payload header. Note it is neutral to specific serialization technologies such as PB, Avro, Thrift
      Hide
      Hadoop QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12501548/rpcPayload2.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 javadoc. The javadoc tool appears to have generated 2 warning messages.

      -1 javac. The patch appears to cause tar ant target to fail.

      -1 findbugs. The patch appears to cause Findbugs (version 1.3.9) to fail.

      +1 release audit. The applied patch does not increase the total number of release audit warnings.

      -1 core tests. The patch failed the unit tests build

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

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

      This message is automatically generated.

      Show
      Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12501548/rpcPayload2.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 javadoc. The javadoc tool appears to have generated 2 warning messages. -1 javac. The patch appears to cause tar ant target to fail. -1 findbugs. The patch appears to cause Findbugs (version 1.3.9) to fail. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed the unit tests build +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/341//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/341//console This message is automatically generated.
      Hide
      Doug Cutting added a comment -

      This looks good.

      If you wanted to make this more easily extensible (HADOOP-7557) then you might change it to have a <<key,value> format, perhaps where keys are vints and values are <vint><payload>. If you put the current three values (kind, op, id) then its size would grow from 6 to perhaps 10 bytes. Alternately you could add a metadata field <count, <key,value>> for future use which would only add a single byte but wouldn't let you ever remove one of the initial three fields.

      Per-call metadata would make it easier to add, e.g., Dapper-style tracing.

      Show
      Doug Cutting added a comment - This looks good. If you wanted to make this more easily extensible ( HADOOP-7557 ) then you might change it to have a <<key,value> format, perhaps where keys are vints and values are <vint><payload>. If you put the current three values (kind, op, id) then its size would grow from 6 to perhaps 10 bytes. Alternately you could add a metadata field <count, <key,value>> for future use which would only add a single byte but wouldn't let you ever remove one of the initial three fields. Per-call metadata would make it easier to add, e.g., Dapper-style tracing.
      Hide
      Sanjay Radia added a comment -

      Doug, not sure of the extra extensibility is necessary. It can be extended by changing the version number on the
      connection header. The same applies to Hadoop-7557 ( i will redo the patch for it).

      Show
      Sanjay Radia added a comment - Doug, not sure of the extra extensibility is necessary. It can be extended by changing the version number on the connection header. The same applies to Hadoop-7557 ( i will redo the patch for it).
      Hide
      Hadoop QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12501746/rpcPayload3.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 javadoc. The javadoc tool did not generate any warning messages.

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

      -1 findbugs. The patch appears to introduce 8 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 .

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

      Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/349//testReport/
      Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/349//artifact/trunk/hadoop-common-project/patchprocess/newPatchFindbugsWarningshadoop-common.html
      Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/349//console

      This message is automatically generated.

      Show
      Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12501746/rpcPayload3.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 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 8 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 . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/349//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/349//artifact/trunk/hadoop-common-project/patchprocess/newPatchFindbugsWarningshadoop-common.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/349//console This message is automatically generated.
      Hide
      Doug Cutting added a comment -

      Sanjay, yes, I agree that extensibility can be added later by changing the version number. Such a change would not be forward compatible. Once extensibility is added then forward-compatible changes to per-request metadata will be possible, but perhaps that's not your goal at this point.

      It seemed to me that, as long as you're re-writing the code that processes headers, and you intend to rewrite it again soon to make it extensible (HADOOP-7557) then we might combine these two steps in order to avoid debugging code that we'll soon discard. But if you'd prefer to implement these steps sequentially that's fine too. Or do I misunderstand the relationship between this and HADOOP-7557?

      Show
      Doug Cutting added a comment - Sanjay, yes, I agree that extensibility can be added later by changing the version number. Such a change would not be forward compatible. Once extensibility is added then forward-compatible changes to per-request metadata will be possible, but perhaps that's not your goal at this point. It seemed to me that, as long as you're re-writing the code that processes headers, and you intend to rewrite it again soon to make it extensible ( HADOOP-7557 ) then we might combine these two steps in order to avoid debugging code that we'll soon discard. But if you'd prefer to implement these steps sequentially that's fine too. Or do I misunderstand the relationship between this and HADOOP-7557 ?
      Hide
      Suresh Srinivas added a comment -
      1. Client.java
        • Comment describing members of Call can be alinged
        • Use rpcResponse instead of rpcResult
        • Make Call#rpcKind, Call#param and Call#id final
        • ParallelCall is alway of kind Writable? Should we get rid of ParallelCall in a separate jira?
        • With rpcKind added to method some of the javadoc @link without rpcKind generate warnings.
      2. RpcPayloadHeader
        • RpcPayloadHeader javadoc could explaing the wire format better.
        • Indentation of readFields is missing a space
        • Is RpcPayloadType a better name for RpcPayloadOperation
        • Need better description of RpcPayloadOperation enums
      3. Server.java
        • Call - a lot of member vars could be final including rpcKind
        • rpcKind is not used in the server.
      Show
      Suresh Srinivas added a comment - Client.java Comment describing members of Call can be alinged Use rpcResponse instead of rpcResult Make Call#rpcKind, Call#param and Call#id final ParallelCall is alway of kind Writable? Should we get rid of ParallelCall in a separate jira? With rpcKind added to method some of the javadoc @link without rpcKind generate warnings. RpcPayloadHeader RpcPayloadHeader javadoc could explaing the wire format better. Indentation of readFields is missing a space Is RpcPayloadType a better name for RpcPayloadOperation Need better description of RpcPayloadOperation enums Server.java Call - a lot of member vars could be final including rpcKind rpcKind is not used in the server.
      Hide
      Sanjay Radia added a comment -

      Updated patch which addresses suresh's feedback

      Show
      Sanjay Radia added a comment - Updated patch which addresses suresh's feedback
      Hide
      Hadoop QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12502106/rpcPayload5.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 javadoc. The javadoc tool did not generate any warning messages.

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

      -1 findbugs. The patch appears to introduce 7 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 .

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

      Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/355//testReport/
      Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/355//artifact/trunk/hadoop-common-project/patchprocess/newPatchFindbugsWarningshadoop-common.html
      Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/355//console

      This message is automatically generated.

      Show
      Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12502106/rpcPayload5.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 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 7 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 . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/355//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/355//artifact/trunk/hadoop-common-project/patchprocess/newPatchFindbugsWarningshadoop-common.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/355//console This message is automatically generated.
      Hide
      Suresh Srinivas added a comment -

      +1 for the patch.

      Show
      Suresh Srinivas added a comment - +1 for the patch.
      Hide
      Sanjay Radia added a comment -

      The findbugs are within trunk.

      Show
      Sanjay Radia added a comment - The findbugs are within trunk.
      Hide
      Hudson added a comment -

      Integrated in Hadoop-Hdfs-trunk-Commit #1323 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1323/)
      HADOOP-7776 Make the Ipc-Header in a RPC-Payload an explicit header (sanjay)

      sradia : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1197885
      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/ProtobufRpcEngine.java
      • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RpcPayloadHeader.java
      • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
      • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/WritableRpcEngine.java
      Show
      Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #1323 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1323/ ) HADOOP-7776 Make the Ipc-Header in a RPC-Payload an explicit header (sanjay) sradia : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1197885 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/ProtobufRpcEngine.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RpcPayloadHeader.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/WritableRpcEngine.java
      Hide
      Hudson added a comment -

      Integrated in Hadoop-Common-trunk-Commit #1249 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1249/)
      HADOOP-7776 Make the Ipc-Header in a RPC-Payload an explicit header (sanjay)

      sradia : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1197885
      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/ProtobufRpcEngine.java
      • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RpcPayloadHeader.java
      • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
      • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/WritableRpcEngine.java
      Show
      Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #1249 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1249/ ) HADOOP-7776 Make the Ipc-Header in a RPC-Payload an explicit header (sanjay) sradia : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1197885 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/ProtobufRpcEngine.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RpcPayloadHeader.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/WritableRpcEngine.java
      Hide
      Hudson added a comment -

      Integrated in Hadoop-Mapreduce-trunk-Commit #1271 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1271/)
      HADOOP-7776 Make the Ipc-Header in a RPC-Payload an explicit header (sanjay)

      sradia : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1197885
      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/ProtobufRpcEngine.java
      • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RpcPayloadHeader.java
      • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
      • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/WritableRpcEngine.java
      Show
      Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #1271 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1271/ ) HADOOP-7776 Make the Ipc-Header in a RPC-Payload an explicit header (sanjay) sradia : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1197885 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/ProtobufRpcEngine.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RpcPayloadHeader.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/WritableRpcEngine.java
      Hide
      Hudson added a comment -

      Integrated in Hadoop-Hdfs-trunk #854 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/854/)
      HADOOP-7776 Make the Ipc-Header in a RPC-Payload an explicit header (sanjay)

      sradia : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1197885
      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/ProtobufRpcEngine.java
      • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RpcPayloadHeader.java
      • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
      • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/WritableRpcEngine.java
      Show
      Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #854 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/854/ ) HADOOP-7776 Make the Ipc-Header in a RPC-Payload an explicit header (sanjay) sradia : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1197885 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/ProtobufRpcEngine.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RpcPayloadHeader.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/WritableRpcEngine.java
      Hide
      Hudson added a comment -

      Integrated in Hadoop-Mapreduce-trunk #888 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/888/)
      HADOOP-7776 Make the Ipc-Header in a RPC-Payload an explicit header (sanjay)

      sradia : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1197885
      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/ProtobufRpcEngine.java
      • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RpcPayloadHeader.java
      • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
      • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/WritableRpcEngine.java
      Show
      Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #888 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/888/ ) HADOOP-7776 Make the Ipc-Header in a RPC-Payload an explicit header (sanjay) sradia : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1197885 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/ProtobufRpcEngine.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RpcPayloadHeader.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/WritableRpcEngine.java
      Hide
      Tsz Wo Nicholas Sze added a comment -

      I have merged this to 0.23.

      Show
      Tsz Wo Nicholas Sze added a comment - I have merged this to 0.23.

        People

        • Assignee:
          Sanjay Radia
          Reporter:
          Sanjay Radia
        • Votes:
          0 Vote for this issue
          Watchers:
          3 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development