Avro
  1. Avro
  2. AVRO-761

Requestor sends protocol text on each handshake under certain circumstances

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.4.1
    • Fix Version/s: 1.5.0
    • Component/s: java
    • Labels:
      None

      Description

      When using a stateless transport such as HTTP and different protocol versions on client and server, Requestor will send the local protocol text on each request once a handshake match is established. For large protocols, this slows things down considerably. The attached patch resolves the issue by resetting the sendLocalText field after a successful handshake.

      1. Requestor.java.patch
        0.5 kB
        Richard Ahrens
      2. AVRO-761.patch
        7 kB
        Doug Cutting

        Activity

        Hide
        Doug Cutting added a comment -

        I committed this.

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

        Unless there are objections, I will commit this soon.

        Show
        Doug Cutting added a comment - Unless there are objections, I will commit this soon.
        Hide
        Doug Cutting added a comment -

        Here's a patch that includes tests.

        RPCContext is altered to expose the entire handshake to plugins, not just the handshake metadata as before. TestProtocolSpecific is altered to check that a given client protocol is only transmitted once, and its TestProtocolHttp is altered to make a second request over a different protocol to trigger the bug. Without the changes to Requestor setting sendLocalText=false TestProtocolHttp now fails, with these changes is passes.

        Show
        Doug Cutting added a comment - Here's a patch that includes tests. RPCContext is altered to expose the entire handshake to plugins, not just the handshake metadata as before. TestProtocolSpecific is altered to check that a given client protocol is only transmitted once, and its TestProtocolHttp is altered to make a second request over a different protocol to trigger the bug. Without the changes to Requestor setting sendLocalText=false TestProtocolHttp now fails, with these changes is passes.
        Hide
        Scott Carey added a comment -

        Marking 1.5.0 for now, it is a non-blocker and may shortly end up 1.5.1 however.

        Show
        Scott Carey added a comment - Marking 1.5.0 for now, it is a non-blocker and may shortly end up 1.5.1 however.
        Hide
        Doug Cutting added a comment -

        Thanks for the patch, Richard.

        It would be good to have a test case for this, but I don't see how to do that easily. Perhaps we could maintain a HandshakeMatch field RPCPlugin, then add a plugin to TestProtocolHttp whose clientFinishConnect() implementation checks that the match for all but the first call is HandshakeMatch.BOTH?

        Show
        Doug Cutting added a comment - Thanks for the patch, Richard. It would be good to have a test case for this, but I don't see how to do that easily. Perhaps we could maintain a HandshakeMatch field RPCPlugin, then add a plugin to TestProtocolHttp whose clientFinishConnect() implementation checks that the match for all but the first call is HandshakeMatch.BOTH?

          People

          • Assignee:
            Doug Cutting
            Reporter:
            Richard Ahrens
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development