Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: None
    • Labels:
      None
    • Release Note:
      RPC can use Avro serialization.

      Description

      Permit RPC protocols to use Avro to serialize requests and responses, so that protocols may better evolve without breaking compatibility.

      1. HADOOP-6170.patch
        17 kB
        Doug Cutting
      2. HADOOP-6170.patch
        16 kB
        Doug Cutting
      3. HADOOP-6170.patch
        13 kB
        Doug Cutting
      4. HADOOP-6170.patch
        13 kB
        Doug Cutting
      5. HADOOP-6170.patch
        13 kB
        Doug Cutting

        Issue Links

          Activity

          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Patch Available Patch Available Open Open
          22d 17h 19m 3 Doug Cutting 18/Sep/09 17:49
          Open Open Patch Available Patch Available
          29d 1h 30m 4 Doug Cutting 18/Sep/09 17:50
          Patch Available Patch Available Resolved Resolved
          56m 42s 1 Doug Cutting 18/Sep/09 18:47
          Resolved Resolved Closed Closed
          340d 2h 51m 1 Tom White 24/Aug/10 21:39
          Tom White made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Doug Cutting made changes -
          Parent HADOOP-6659 [ 12460169 ]
          Issue Type New Feature [ 2 ] Sub-task [ 7 ]
          Robert Chansler made changes -
          Release Note RPC can use Avro serialization.
          Hide
          Robert Chansler added a comment -

          Editorial pass over all release notes prior to publication of 0.21.

          Show
          Robert Chansler added a comment - Editorial pass over all release notes prior to publication of 0.21.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk #102 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/102/)
          . Add facility to tunnel Avro RPCs through Hadoop RPCs.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk #102 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/102/ ) . Add facility to tunnel Avro RPCs through Hadoop RPCs.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #39 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/39/)
          . Add facility to tunnel Avro RPCs through Hadoop RPCs.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #39 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/39/ ) . Add facility to tunnel Avro RPCs through Hadoop RPCs.
          Doug Cutting made changes -
          Resolution Fixed [ 1 ]
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hide
          Doug Cutting added a comment -

          I just committed this.

          Show
          Doug Cutting added a comment - I just committed this.
          Hide
          Tom White added a comment -

          +1

          Show
          Tom White added a comment - +1
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 7 new or modified tests.

          +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 does not introduce any new Findbugs warnings.

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

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

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/52/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/52/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/52/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/52/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/12420048/HADOOP-6170.patch against trunk revision 816703. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. +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 does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/52/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/52/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/52/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/52/console This message is automatically generated.
          Doug Cutting made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Doug Cutting made changes -
          Attachment HADOOP-6170.patch [ 12420048 ]
          Hide
          Doug Cutting added a comment -

          Fix Ivy settings to use library.properties for consistency with other dependencies.

          Show
          Doug Cutting added a comment - Fix Ivy settings to use library.properties for consistency with other dependencies.
          Doug Cutting made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 7 new or modified tests.

          +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 does not introduce any new Findbugs warnings.

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

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

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/45/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/45/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/45/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/45/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/12419947/HADOOP-6170.patch against trunk revision 816409. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. +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 does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/45/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/45/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/45/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/45/console This message is automatically generated.
          Doug Cutting made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Doug Cutting made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Doug Cutting made changes -
          Attachment HADOOP-6170.patch [ 12419947 ]
          Hide
          Doug Cutting added a comment -

          > Do add some JavaDoc per class.

          Done.

          > Can a single RPC server satisfy multiple protocols?

          Yes, that should work. The client and servers protocols are both determined through reflection. So long as the server implements the subset of methods that the client calls then all's well. Avro doesn't validate the protocol name in RPC, just the method name. Dunno if that's a bug or a feature! (I think this is the same loophole that Hadoop RPC uses.)

          > Typo.

          Fixed.

          > AvroTestProtocol is generated code, yes? Or does ReflectData.getProtocol()
          figure it out from reflection and paranamer data?

          The latter. This current patch only supports reflection-based RPC, since it's intended to be a stepping stone for transition from Hadoop RPC, which is reflection-based.

          > Total nit: JUnit prefers the expected argument on the left.

          Fixed.

          > Eclipse tells me this is unused.

          Added a comment.

          > getRemoteName()

          { return "remote"; }

          Good catch! That was a bug. This is used by Avro to cache the remote protocol. It should be the remote host's address. Fixed.

          I also took the opportunity to upgrade from Avro 1.0 to Avro 1.1, which required a few minor changes to the io/serializer/avro stuff.

          Show
          Doug Cutting added a comment - > Do add some JavaDoc per class. Done. > Can a single RPC server satisfy multiple protocols? Yes, that should work. The client and servers protocols are both determined through reflection. So long as the server implements the subset of methods that the client calls then all's well. Avro doesn't validate the protocol name in RPC, just the method name. Dunno if that's a bug or a feature! (I think this is the same loophole that Hadoop RPC uses.) > Typo. Fixed. > AvroTestProtocol is generated code, yes? Or does ReflectData.getProtocol() figure it out from reflection and paranamer data? The latter. This current patch only supports reflection-based RPC, since it's intended to be a stepping stone for transition from Hadoop RPC, which is reflection-based. > Total nit: JUnit prefers the expected argument on the left. Fixed. > Eclipse tells me this is unused. Added a comment. > getRemoteName() { return "remote"; } Good catch! That was a bug. This is used by Avro to cache the remote protocol. It should be the remote host's address. Fixed. I also took the opportunity to upgrade from Avro 1.0 to Avro 1.1, which required a few minor changes to the io/serializer/avro stuff.
          Hide
          Philip Zeyliger added a comment -

          I took a look. Overall, looks good. I'm sufficiently behind on my Avro-fu that I
          had a hard time following some of the magic.

          I'm writing down below what my current understanding is. Let me know
          where I'm totally wrong. Do add some JavaDoc per class.

          TunnelProtocol is the meta-interface that's used by Hadoop IPC.

          BufferListWritable is used by Hadoop IPC to ship
          the Avro byte-stream around. Its serialization format is:

          data := size (buffer)[size]
          buffer := len (bytes)[len]
          size, len are writable integers
          bytes are raw bytes
          x[count] indicates count repetitions of x.
          

          ClientTranceiver delegates shipping of bytes via Hadoop IPC.
          ServerTranceiver doesn't do much of anything except store
          the response data. New instances of it are created at every
          call in TunnelResponder.

          TunnelResponder implements the Hadoop IPC (TunnelProtocol);
          it's the thing running on the server, and converts calls
          into Avro RPC calls. This class would be clearer if, instead
          of extending ReflectResponder, you kept a private ReflectResponder,
          and called that explicitly in call. Then you would rename
          TunnelResponder to TunnelProtocolImpl.

          Can a single RPC server satisfy multiple protocols? Does that
          work with AvroRPC? I don't think it can right now, but I think
          that's necessary, since several daemons implement a handful of
          protocols.

          Some specific notes:

          versioning features to for inter-Java RPCs.

          Typo.

          AvroTestProtocol is generated code, yes? Or does ReflectData.getProtocol()
          figure it out from reflection and paranamer data? If the former,
          should the source schema be checked in? Should the generation
          be done in build.xml?

          assertEquals(intResult, 3);

          Total nit: JUnit prefers the expected argument on the left.

          public BufferListWritable() {}

          Eclipse tells me this is unused. Might be worth
          a quick comment indicating that it's required because
          Writables get instantiated via reflection.

          getRemoteName() { return "remote"; }

          Should these be customized to the Protocol being used?

          Cheers,

          – Philip

          Show
          Philip Zeyliger added a comment - I took a look. Overall, looks good. I'm sufficiently behind on my Avro-fu that I had a hard time following some of the magic. I'm writing down below what my current understanding is. Let me know where I'm totally wrong. Do add some JavaDoc per class. TunnelProtocol is the meta-interface that's used by Hadoop IPC. BufferListWritable is used by Hadoop IPC to ship the Avro byte-stream around. Its serialization format is: data := size (buffer)[size] buffer := len (bytes)[len] size, len are writable integers bytes are raw bytes x[count] indicates count repetitions of x. ClientTranceiver delegates shipping of bytes via Hadoop IPC. ServerTranceiver doesn't do much of anything except store the response data. New instances of it are created at every call in TunnelResponder. TunnelResponder implements the Hadoop IPC (TunnelProtocol); it's the thing running on the server, and converts calls into Avro RPC calls. This class would be clearer if, instead of extending ReflectResponder, you kept a private ReflectResponder, and called that explicitly in call. Then you would rename TunnelResponder to TunnelProtocolImpl. Can a single RPC server satisfy multiple protocols? Does that work with AvroRPC? I don't think it can right now, but I think that's necessary, since several daemons implement a handful of protocols. Some specific notes: versioning features to for inter-Java RPCs. Typo. AvroTestProtocol is generated code, yes? Or does ReflectData.getProtocol() figure it out from reflection and paranamer data? If the former, should the source schema be checked in? Should the generation be done in build.xml? assertEquals(intResult, 3); Total nit: JUnit prefers the expected argument on the left. public BufferListWritable() {} Eclipse tells me this is unused. Might be worth a quick comment indicating that it's required because Writables get instantiated via reflection. getRemoteName() { return "remote"; } Should these be customized to the Protocol being used? Cheers, – Philip
          Hide
          Doug Cutting added a comment -

          Can another committer please review this? Thanks!

          Show
          Doug Cutting added a comment - Can another committer please review this? Thanks!
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 7 new or modified tests.

          +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 does not introduce any new Findbugs warnings.

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

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

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/630/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/630/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/630/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/630/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/12417892/HADOOP-6170.patch against trunk revision 808415. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. +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 does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/630/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/630/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/630/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/630/console This message is automatically generated.
          Doug Cutting made changes -
          Attachment HADOOP-6170.patch [ 12417899 ]
          Hide
          Doug Cutting added a comment -

          Ignore the warning rather than add bogus code to make it go away.

          Show
          Doug Cutting added a comment - Ignore the warning rather than add bogus code to make it go away.
          Doug Cutting made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Doug Cutting made changes -
          Attachment HADOOP-6170.patch [ 12417892 ]
          Hide
          Doug Cutting added a comment -

          New patch that fixes lint warnings.

          Show
          Doug Cutting added a comment - New patch that fixes lint warnings.
          Doug Cutting made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 7 new or modified tests.

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

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

          -1 findbugs. The patch appears to introduce 1 new Findbugs warnings.

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

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

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/628/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/628/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/628/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/628/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/12417826/HADOOP-6170.patch against trunk revision 807753. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The applied patch generated 119 javac compiler warnings (more than the trunk's current 116 warnings). -1 findbugs. The patch appears to introduce 1 new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/628/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/628/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/628/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/628/console This message is automatically generated.
          Doug Cutting made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Doug Cutting made changes -
          Attachment HADOOP-6170.patch [ 12417826 ]
          Andrew Purtell made changes -
          Field Original Value New Value
          Link This issue relates to HBASE-794 [ HBASE-794 ]
          Hide
          Doug Cutting added a comment -

          To implement this, an Avro Transciever can be implemented using Hadoop's Client and Server classes. The request and response Writable would contain just List<ByteBuffer>.

          Show
          Doug Cutting added a comment - To implement this, an Avro Transciever can be implemented using Hadoop's Client and Server classes. The request and response Writable would contain just List<ByteBuffer>.
          Doug Cutting created issue -

            People

            • Assignee:
              Doug Cutting
              Reporter:
              Doug Cutting
            • Votes:
              0 Vote for this issue
              Watchers:
              19 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development