Avro
  1. Avro
  2. AVRO-271

InProcessTranceiver: connect RPCs without going through any sockets

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3.0
    • Component/s: java
    • Labels:
      None

      Description

      For testing (both Avro itself, and code that uses
      Avro servers) it's sometimes handy to connect the RPCs without
      even a socket. This implementation of a Transceiver does just that.
      (This can, for users, both avoid the overhead of using sockets, and,
      in debugging, let users see stack traces that have both client
      and server code in one thread.)

      1. AVRO-271.patch.txt
        5 kB
        Philip Zeyliger
      2. AVRO-271.patch.txt
        4 kB
        Philip Zeyliger

        Issue Links

          Activity

          Hide
          Philip Zeyliger added a comment -

          Attaching a patch.

          The code itself is pretty trivial. I think it's important that this exist in the src/java tree (and not the test/src/java) tree, because it's something that can be used for clients of AVRO testing their own code. For example, when Hadoop sets up a MiniCluster, this could be used.

          This was my first foray into the Transceiver initerface (really, abstract class). Am I right that the interface would need to be changed to support a single transceiver being used to process multiple RPCs concurrently? I was thinking of having InProcessTransceiver backed by an Executor, but it didn't make sense. Naively, you could change Transceiver to be an interface with just getRemoteName(), close(), and transceive(), and the fact that many implementations share a synchronized transceive would be an implementation detail. To be clear, I don't wish this JIRA to become the one where we maybe change Transceiver.java.

          – Philip

          Show
          Philip Zeyliger added a comment - Attaching a patch. The code itself is pretty trivial. I think it's important that this exist in the src/java tree (and not the test/src/java) tree, because it's something that can be used for clients of AVRO testing their own code. For example, when Hadoop sets up a MiniCluster, this could be used. This was my first foray into the Transceiver initerface (really, abstract class). Am I right that the interface would need to be changed to support a single transceiver being used to process multiple RPCs concurrently? I was thinking of having InProcessTransceiver backed by an Executor, but it didn't make sense. Naively, you could change Transceiver to be an interface with just getRemoteName(), close(), and transceive(), and the fact that many implementations share a synchronized transceive would be an implementation detail. To be clear, I don't wish this JIRA to become the one where we maybe change Transceiver.java. – Philip
          Hide
          Doug Cutting added a comment -

          > Am I right that the interface would need to be changed to support a single transceiver being used to process multiple RPCs concurrently?

          That's not the intent. It's intended to be bound to a particular destination, and proabably (when we get our heads around authentication) a particular user, but that user might reasonably have multiple requests outstanding to that destination using a single transceiver. In particular, a client proxy object has a single transceiver. A proxy should be thread safe, no?

          Show
          Doug Cutting added a comment - > Am I right that the interface would need to be changed to support a single transceiver being used to process multiple RPCs concurrently? That's not the intent. It's intended to be bound to a particular destination, and proabably (when we get our heads around authentication) a particular user, but that user might reasonably have multiple requests outstanding to that destination using a single transceiver. In particular, a client proxy object has a single transceiver. A proxy should be thread safe, no?
          Hide
          Philip Zeyliger added a comment -

          A proxy should be thread safe, no?

          I agree.

          Thinking about this a bit more, I think the problem is that readBuffers() and writeBuffers() shouldn't be public. If they're public, there's no way to correlate two concurrent requests with the correct buffers. tranceive() works fine, because it's a single method, and the Transceiver might be doing the coordination inside.

          – Philip

          Show
          Philip Zeyliger added a comment - A proxy should be thread safe, no? I agree. Thinking about this a bit more, I think the problem is that readBuffers() and writeBuffers() shouldn't be public. If they're public, there's no way to correlate two concurrent requests with the correct buffers. tranceive() works fine, because it's a single method, and the Transceiver might be doing the coordination inside. – Philip
          Hide
          Doug Cutting added a comment -

          > the problem is that readBuffers() and writeBuffers() shouldn't be public.

          These were intended to be used by servers to read requests and write responses. But my hope of writing a generic server that's passed a Transceiver hasn't worked out in practice, even when servers process requests synchronously, and it definitely wouldn't work for an async server. So removing readBuffers() and writeBuffers() altogether might make sense, and declaring that Transceiver is for client-side use only.

          InProcessTransceiver can simply implement transceive() to directly invoke Responder#respond().

          Also, what do you think of renaming InProcessTransceiver to be LocalTransceiver?

          Show
          Doug Cutting added a comment - > the problem is that readBuffers() and writeBuffers() shouldn't be public. These were intended to be used by servers to read requests and write responses. But my hope of writing a generic server that's passed a Transceiver hasn't worked out in practice, even when servers process requests synchronously, and it definitely wouldn't work for an async server. So removing readBuffers() and writeBuffers() altogether might make sense, and declaring that Transceiver is for client-side use only. InProcessTransceiver can simply implement transceive() to directly invoke Responder#respond(). Also, what do you think of renaming InProcessTransceiver to be LocalTransceiver?
          Hide
          Philip Zeyliger added a comment -

          > So removing readBuffers() and writeBuffers() altogether might make sense, and declaring that Transceiver is for client-side use only.

          Makes sense. Let's do a separate JIRA for that.

          > InProcessTransceiver can simply implement transceive() to directly invoke Responder#respond().

          Indeed; makes it even simpler.

          > Also, what do you think of renaming InProcessTransceiver to be LocalTransceiver?

          I like it. Done.

          Show
          Philip Zeyliger added a comment - > So removing readBuffers() and writeBuffers() altogether might make sense, and declaring that Transceiver is for client-side use only. Makes sense. Let's do a separate JIRA for that. > InProcessTransceiver can simply implement transceive() to directly invoke Responder#respond(). Indeed; makes it even simpler. > Also, what do you think of renaming InProcessTransceiver to be LocalTransceiver? I like it. Done.
          Hide
          Doug Cutting added a comment -

          I just committed this. Thanks Philip!

          Show
          Doug Cutting added a comment - I just committed this. Thanks Philip!

            People

            • Assignee:
              Philip Zeyliger
              Reporter:
              Philip Zeyliger
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development