All contributions to the effort are much appreciated. I'm confident all of your efforts will result in a final implementation that is very high quality.
I apologize that no Avro committer has had time to review AVRO-405 and provide feedback. It appears that the three contributors involved here know more about Netty than any committer, and it would be very helpful if you can resolve your differences and submit a patch that all of you can agree is a good implementation. I am not an expert on the RPC side, but I can help provide some guidelines for the public facing API: * The public facing API should be as clear and simple to use as possible. Don't worry about trying to 'look' or 'feel' like any prior API, Avro is young and the APIs are evolving -- especially in this area. Obviously, change for the sake of change is pointless, but if a change simplifies use or provides key new capability don't let precedent get in the way. If you were to write a 'how-to' page on our Wiki for this api, how simple would it be? Could someone with little prior Avro experience and no Netty experience get up and running fast? Is it simple enough that the Javadoc on public classes is enough for most users? If other parts of Avro need to be changed to reach your goals, don't let that get in the way. We'll review those changes as well. -Scott On Jun 27, 2010, at 12:43 AM, James Todd wrote: > i do not believe alot of change is required. it should be quite trivial, > actually, given my understanding of the handshake processing. > > i would be happy to work together to construct the optimal solution and > intend to become a client of the resulting work. > > - james > > On Sat, Jun 26, 2010 at 11:37 PM, harry wang <[email protected]> wrote: > >> Sorry I'm a newbie to the Avro, and not familiar with the design philosophy >> of the Avro internals very much. The only thing I want to do is to use >> Avro >> with Netty to improve the network communication performance in my project, >> so I wrote these codes to make it work. It looks obtrusively that I submit >> my implementation patch before we discuss deeply. How can I withdraw my >> patch attachment? There are still bugs in it. I fixed them in my repository >> at http://github.com/coolwhy/avro-rpc-on-netty. >> Your direction maybe right if the Responder could be resolved from the >> handshake protocol of client and server. But it seems that a lot of things >> in Avro should be changed? I'm not sure. Go ahead please and I would >> appreciate your work if you succeed. Thank you :) >> >> - harry >> >> On Sat, Jun 26, 2010 at 2:21 PM, James Todd <[email protected]> >> wrote: >> >>> personally and as a potential user of the api, i prefer the approach i >> have >>> started for obvious reasons. >>> >>> for your approach, what is the reconciliation process for when the >>> user-provided-responder differs from that as specified in the >>> request header? and again the question, why require the user to specify a >>> responder when the request handshake includes all >>> the necessary data to make such a decision? perhaps this is a detail but >> to >>> me it is a key design consideration. >>> >>> i do believe the proper solution is to internalize the responder delegate >>> based on inspection of the request handshake. >>> >>> now, as to the other issues, ie more-vs-fewer classes implementing the >>> various steps in the pipeline ... i consider that a moot >>> issue if the implementations are functionally equivalent and can consider >>> rolling up the implementations to one-uber implementation >>> fine but not necessarily required. again, a moot issue imo. >>> >>> lastly i have had this patch up for review for quite some time now w/ no >>> significant progress other then several folks reporting it >>> works and have submitted patches for subtle bugs. i intended to continue >>> this design approach given the patch is currently >>> additive to the avro distribution (ie an optional extension if not added >> to >>> avro proper, which i would very much like to see happen). >>> now, the additional bit of work i have mentioned will require internal >> avro >>> implementation changes but they are isolated and not >>> disruptive (this has been discussed on #avro). >>> >>> as such, the patch i have provided is incomplete but the remaining work >> is >>> a >>> known level of effort. then again, it is stuck. >>> >>> your patch likely works and in the spirit of seeing progress made i do >> not >>> wish to stand in the way. i do not believe i can give >>> your patch a +1 (fwiw) given my stance and work provided thus far. i have >>> had a difficult time obtaining reviews for this work >>> thus far and feel a bit disappointed about that ... but i share the blame >>> given responsibilities of late (work/etc) and the fact i >>> have one remaining documented TODO to complete the intended design. >>> >>> so, here we are. i plan to continue the work i have started and intend to >>> use the results (we are keen on using it at work) >>> with the remaining work being: >>> >>> delegate to the specified handler for the designated protocol as >> inferred >>> from the handshake >>> >>> implement bruce's improved protocol >>> >>> update netty >>> >>> (note: as of last week my weekend-time has freed up significantly and i >>> intend to finish this work) >>> >>> if you would like to proceed with an alternative implementation that is >>> fine >>> with me, albeit suboptimal imo, but that's the way >>> things happen. i can not offer my +1 vote/review fwiw. do feel free to >>> continue your work as it would be great to continue the >>> work in this regard. >>> >>> best, >>> >>> - james >>> >>> >>> On Fri, Jun 25, 2010 at 9:39 PM, harry wang <[email protected]> wrote: >>> >>>> en, I hope we can reach a consistent solution. I prefer the behavior of >>> the >>>> solution is the same as Avro Server/Transceiver pair because they are >>> used >>>> now in the release package. The same behavior to the API customers is >>>> important. If the Server/Transceiver design is not good enough, we guys >>>> should improve all the related classes later. How do you think about >> it? >>>> >>>> - harry >>>> >>>> On Sat, Jun 26, 2010 at 2:58 AM, James Todd <[email protected]> >>>> wrote: >>>> >>>>> saw that. i will dive in. >>>>> >>>>> i am curious as to your thoughts regarding my response. i think the >>>>> differences are 1) philosophical [eg simple external api as a >> principal >>>>> objective] and 2) tactical [eg internal implementation details]. >>>>> >>>>> optimally, we can collectively meld the ideas for an overall improved >>>>> solution. >>>>> >>>>> there is still outstanding work regarding the responder delegation >> work >>>>> that >>>>> is assumed to be follow on work for patch i provided. >>>>> >>>>> thoughts? >>>>> >>>>> - james >>>>> >>>>> On Fri, Jun 25, 2010 at 11:46 AM, harry wang <[email protected]> >>> wrote: >>>>> >>>>>> hi, I just attached my implementation patch as another choice for >>> trial >>>>> at >>>>>> https://issues.apache.org/jira/browse/AVRO-405. :) >>>>>> Maybe we could get a better result in the end. >>>>>> >>>>>> regards >>>>>> >>>>>> - harry >>>>>> >>>>>> On Sat, Jun 26, 2010 at 2:24 AM, James Todd < >> [email protected]> >>>>>> wrote: >>>>>> >>>>>>> hey harry - >>>>>>> >>>>>>> glad to hear there is functional parity :) >>>>>>> >>>>>>> it will be good to get this initial issue in one way or another. >>>>>>> >>>>>>> i opted to leverage the netty internals to manage/contain the >>>> discreet >>>>>>> steps >>>>>>> in the pipeline but admittedly they are trivial and can in all >>>>> likelihood >>>>>>> be >>>>>>> rolled up. i am keen on implementing bruce's proposed protocol >> and >>>>>> perhaps >>>>>>> this objective led me to this design. regardless it is solely >>>> internal >>>>>> and >>>>>>> up for refactoring. >>>>>>> >>>>>>> there is one significant TODO in the patch i provided which is to >>>>>>> internally >>>>>>> determine the relevant responder by inspecting the handshake data >>> and >>>>>>> delegating accordingly. that is work that is assumed >>>>>>> to go along with this patch and work worth doing imo, as the data >>> is >>>>> all >>>>>>> available and it streamlines/simplifies the external api. >>>>>>> >>>>>>> to summarize, error on the side of simplest possible external api >>>>> (noting >>>>>>> the afore mentioned responder delegation work) and allow for >>>> (possibly >>>>>>> speculative) implementation variability for the internal details. >>>>>>> >>>>>>> i also didn't necessarily strive to align w/ other >> implementations >>>>>>> (ie SocketTransceiver/SocketServer or HttpTransceiver/HttpServer) >>> as >>>> i >>>>>>> didn't see that as significantly advantageous to do so. guess i >>> could >>>>> be >>>>>>> wrong. >>>>>>> >>>>>>> thoughts? >>>>>>> >>>>>>> best, >>>>>>> >>>>>>> - james >>>>>>> >>>>>>> On Fri, Jun 25, 2010 at 4:23 AM, harry wang <[email protected]> >>>> wrote: >>>>>>> >>>>>>>> Hi james, after studying your works, I find that our basic idea >>> is >>>>>> alike >>>>>>>> but >>>>>>>> our implementation is a little different. It appears my design >> is >>>>>> simpler >>>>>>>> than yours. The following is the comparison: >>>>>>>> >>>>>>>> 1, my design only has 4 files: NettyFrameDecoder.java, >>>>>>>> NettyFrameEncoder.java, NettyServer.java and >>> NettyTransceiver.java, >>>>> in >>>>>>>> which >>>>>>>> Encoder/Decoder classes transform data structure between >>>>>> List<ByteBuffer> >>>>>>>> (need by Responder) and ChannelBuffer (need by Netty), Server >>> class >>>>> as >>>>>> a >>>>>>>> server and Transceiver as a client. The design is more similar >>> with >>>>>>>> SocketServer and SocketTransceiver, so does the usage. i.e. >>>>>>>> >>>>>>>> // server >>>>>>>> Responder responder = new SpecificResponder(Mail.class, >>> new >>>>>>>> MailImpl()); >>>>>>>> Server server = new NettyServer(responder, new >>>>>>>> InetSocketAddress(0)); >>>>>>>> Thread.sleep(1000); // waiting for server startup >>>>>>>> >>>>>>>> // client >>>>>>>> int serverPort = server.getPort(); >>>>>>>> Transceiver transceiver = new NettyTransceiver(new >>>>>>>> InetSocketAddress(serverPort)); >>>>>>>> Mail proxy = >> (Mail)SpecificRequestor.getClient(Mail.class, >>>>>>>> transceiver); >>>>>>>> >>>>>>>> Message msg = new Message(); >>>>>>>> msg.to = new Utf8("wife"); >>>>>>>> msg.from = new Utf8("husband"); >>>>>>>> msg.body = new Utf8("I love you!"); >>>>>>>> >>>>>>>> try { >>>>>>>> Utf8 result = proxy.send(msg); >>>>>>>> System.out.println("Result: " + result); >>>>>>>> } finally { >>>>>>>> transceiver.close(); >>>>>>>> server.close(); >>>>>>>> } >>>>>>>> >>>>>>>> 2, your design has about 10 files because you use more >> handlers >>> in >>>>> the >>>>>>>> pipeline and more top level classes such as client/server >>>>>>> PipelineFactory. >>>>>>>> The biggest difference is that your client and server class >>> design >>>> is >>>>>> not >>>>>>>> similar with SocketTransceiver/SocketServer or >>>>>> HttpTransceiver/HttpServer >>>>>>>> pair. And the usage method is : >>>>>>>> >>>>>>>> // server >>>>>>>> netSocketAddress address = new InetSocketAddress(port); >>>>>>>> AvroServer server = new AvroServer(address); // where is >>> the >>>>>>>> Responder instance ? >>>>>>>> >>>>>>>> // client >>>>>>>> InetSocketAddress address = new InetSocketAddress(port); >>>>>>>> AvroClient client = new AvroClient(address); >>>>>>>> Message message = createMessage(to, from, body); >>>>>>>> String response = client.dispatch(message); // not use >> the >>>>> Proxy >>>>>>>> pattern >>>>>>>> System.out.println("response: " + response); >>>>>>>> client.dispose(); >>>>>>>> >>>>>>>> In your design there is a problem that you create a specific >>>>> Responder >>>>>>>> instance using specific protocol in AvroServerHandler which >> could >>>> not >>>>>> be >>>>>>>> reused in other circumstances. >>>>>>>> >>>>>>>> So, I think my design is more close to the Avro's way. How do >> you >>>>> think >>>>>>>> about it? and anyone else? >>>>>>>> >>>>>>>> - harry >>>>>>>> >>>>>>>> On Fri, Jun 25, 2010 at 11:47 AM, James Todd < >>>> [email protected] >>>>>> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> the latest/greatest patch against AVRO-405 is: >>>>>>>>> >>>>>>> >>>>> >>> https://issues.apache.org/jira/secure/attachment/12441447/AVRO-405.patch >>>>>>>>> >>>>>>>>> it's a merge of bo shin's and my work. >>>>>>>>> >>>>>>>>> there is more to do here, should be summarized in the >> comments >>>>> iirc, >>>>>>> but >>>>>>>> it >>>>>>>>> would be great to get this initial spike done and build >>>>>>>>> on from that point. >>>>>>>>> >>>>>>>>> best, >>>>>>>>> >>>>>>>>> - james >>>>>>>>> >>>>>>>>> On Thu, Jun 24, 2010 at 8:04 PM, harry wang < >> [email protected] >>>> >>>>>> wrote: >>>>>>>>> >>>>>>>>>> OK. But it seems that someone else has already made a >>> netty-rpc >>>>>>> patch. >>>>>>>> I >>>>>>>>>> would like to see if my work could be merged into it. >>>>>>>>>> >>>>>>>>>> - harry >>>>>>>>>> >>>>>>>>>> On Fri, Jun 25, 2010 at 2:50 AM, Doug Cutting < >>>>> [email protected]> >>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> This would make a great contribution! >>>>>>>>>>> >>>>>>>>>>> Can you please attach it as a patch to an issue in Jira? >>>>>>>>>>> >>>>>>>>>>> Thanks, >>>>>>>>>>> >>>>>>>>>>> Doug >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On 06/24/2010 11:29 AM, harry wang wrote: >>>>>>>>>>> >>>>>>>>>>>> hi, I have implemented the Avro RPC Server and >> Transceiver >>>>> using >>>>>>>>> Netty. >>>>>>>>>> If >>>>>>>>>>>> anyone is interested in it, you can look at >>>>>>>>>>>> http://github.com/coolwhy/avro-rpc-on-netty. Any >>> suggestion >>>>> is >>>>>>>>> welcome. >>>>>>>>>>>> Thanks! >>>>>>>>>>>> >>>>>>>>>>>> - harry >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >>
