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 > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
