Re: [infinispan-dev] Hot Rod - pt2

> Why do you have OpCode in your > response header? Surely this is redundant? If > the client is sync, it knows what it sent. If it is > async, it has a message ID. >>>>> True, I fixed that. I don't think it's a significant issue, and it is sort of redundant in this case because you have the status response, however, it isn't completely clear cut. When I implemented a similar protocol I kept the response code.

It simplifies what you need to track within a request/response cycle, and allows for easy extensibility. For example, on error I return a different response type (a different op code) which causes the response to be parsed by a different message decoder.

I also made it easy to register custom requests and responses, and this is facilitated by having distinct op codes for each.

Alex --- On Wed, 12/16/09, Galder Zamarreno&lt;galder(a)redhat.com&gt; wrote: > From: Galder Zamarreno&lt;galder(a)redhat.com&gt; > Subject: Re: [infinispan-dev] Hot Rod - pt2 > To: infinispan-dev(a)lists.jboss.org > Date: Wednesday, December 16, 2009, 8:16 AM > > > On 12/16/2009 02:53 PM, Manik Surtani wrote: >> >> On 16 Dec 2009, at 13:38, Galder Zamarreno wrote: >> >>> >>> >>> On 12/16/2009 12:14 PM, Manik Surtani wrote: >>>> >>>> On 16 Dec 2009, at 09:26, Galder Zamarreno > wrote: >>>> >>>>> >>>>> >>>>> On 12/15/2009 01:13 PM, Manik Surtani > wrote: >>>>>> A few comments: >>>>>> >>>>>> - Why do you have OpCode in your > response header? Surely this is redundant? If > the client is sync, it knows what it sent. If it is > async, it has a message ID. >>>>> >>>>> True, I fixed that. >>>>> >>>>>> - 'Not So Dumb' and 'Clever' response > headers should be optional? Surely this stuff is only > sent when there is a topology change? We also may need > some extra info here - how does the back-end know to send > this info? If a client hits different back-end nodes, > and there is a topology change, how does Node A decide that > it should not bother with topology info since the client > already hit Node B after the topo change and has the new > topology map? Perhaps a TopologyVersion (== JGroups > View ID) should be sent back with any topo map, and the > client would send it's current TopologyVersion with every > request (non-dumb clients only)? Could be a vlong... >>>>> >>>>> Yeah, as you and Mircea suggest, clients > sending the view id could help >>>>> better manage size of responses. >>>>> >>>>> Something to note here is that not all > cluster view changes necessarily >>>>> involve a client side topology view > change, cos if you have N infinispan >>>>> nodes in the cluster, you could have M > running hot rod server where M<= >>>>> N. So, Hot rod will need to figure out > when there's been a view change >>>>> in the cluster that involves the addition > or removal of a hot rod >>>>> running Infinispan instance (similar thing > to what happens when a >>>>> clustered EJB is deployed, the target list > for that clustered EJB gets >>>>> updated). >>>> >>>> Yes. >>>> >>>>> So, bearing in mind this, could we just > use the JGroups view id for >>>>> this? AFAIK, it's 0 based long but > shouldn't cause problems with >>>>> complete cluster restarts. If the whole > cluster gets restarted, existing >>>>> connections will be closed and at that > point, clients could revert back >>>>> to trying to connect one of their known > hot rod servers and pass -1 as >>>>> view id which means that I have no view, > so the responding server would >>>>> send back the hot rod cluster view. >>>> >>>> Provided the client knows the servers have all > been restarted. >>>> >>>> I think a better approach is that the client > sends its last known topology version (vClient). The > server node it connects to compares vClient with vServer > (the version that the servers are aware of). if > vClient != vServer then resend topology map. >>> >>> I added that already this morning to the request > header :) >>> http://community.jboss.org/wiki/HotRodProtocol >>> >>> Not-so-dumb and clever clients send their latest > viewId to the server >>> and the server compares would compare it and if > different, send the new >>> topology. >>> >>> Note that in the response, I had added a "View > change marker" field so >>> that the client knows whether a view change > follows or not. The reason I >>> had done this is because I didn't think the client > could simply compare >>> the vServer with the vClient and decide whether > more needs to be read or >>> not. >> >> No, the server decides whether the Topo map is out of > date. If so, it sends a new one. The client just > applies the new one if a new one is sent. > > Indeed, but this was about the client knowing that a new > topo has been > sent to it. I mean, how does the client know that server > has actually > sent back a new topology? I was using that field to > indicate to the > client: "hey, a new view follows the head after this field > read". I can > then read the vServer sent...etc. > >> >>> You could have an scenario where a client starts > up and sends 3 request >>> in pararell, all with vClient=-1 because no > replies have been received >>> from hot rod server. When the server replied to > those 3 req, it would >>> have sent for example, vServer=4. The first > response to be processed >>> would have update the local vClient to 4, but the > other two responses >>> would still contain the vServer=4. So, they would > somehow need to read, >>> or discard the response. >>> >>> And after writing this and thinking it again, the > "View change marker" >>> is not be necessary. If vClient and vServer are > equals, the client knows >>> the total length of the body so it can read it and > discard it directly. > > Hmmmm, but thinking through this again, the total body > includes other > stuff. So I do need that marker unless the header comes > with a total > number of bytes in header, in which case yeah, I can > discard the > remaining bytes from the header and ignore the topology > information cos > I have already applied it. > >>> >>>> >>>>> I'll add this to the wiki. >>>>> >>>>>> >>>>>> Cheers >>>>>> Manik >>>>>> >>>>>> >>>>>> On 14 Dec 2009, at 20:08, Galder > Zamarreno wrote: >>>>>> >>>>>>> Hi all, >>>>>>> >>>>>>> Re: http://community.jboss.org/wiki/HotRodProtocol >>>>>>> >>>>>>> I've updated the wiki with the > following stuff: >>>>>>> - Renamed replaceIfEquals to > replaceIfUnmodified >>>>>>> - Added remove and > removeIfUnmodified. >>>>>>> - Added containsKey command. >>>>>>> - Added getWithCas command so that > cas value can be returned. I decided >>>>>>> for a separate command rather than > adding cas to get return because you >>>>>>> don't always want cas to be > returned. Having a separate command makes >>>>>>> better use of network bandwith. >>>>>>> - Added stats command. JMX > attributes are basically accessible through >>>>>>> this, including cache size. >>>>>>> - Added error handling section and > updated status codes. >>>>>>> >>>>>>> Note that Mircea added some > interesting comments and I replied to them >>>>>>> directly in the wiki. >>>>>>> >>>>>>> Still remaining to add: >>>>>>> - Commands: putForExternalRead > evict, clear, version, name and quite >>>>>>> commands. >>>>>>> - Passing flags. >>>>>>> >>>>>>> Regards, >>>>>>> >>>>>>> p.s. Updating this has been quite > a struggle due to F12 + FF 3.5.5 >>>>>>> crashing at least 5 times, plus > parts of the wiki dissapearing after >>>>>>> publishing them! >>>>>>> -- >>>>>>> Galder Zamarreño >>>>>>> Sr. Software Engineer >>>>>>> Infinispan, JBoss Cache >>>>>>> > _______________________________________________ >>>>>>> infinispan-dev mailing list >>>>>>> infinispan-dev(a)lists.jboss.org >>>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>>>>> >>>>>> -- >>>>>> Manik Surtani >>>>>> manik(a)jboss.org >>>>>> Lead, Infinispan >>>>>> Lead, JBoss Cache >>>>>> http://www.infinispan.org >>>>>> http://www.jbosscache.org >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> > _______________________________________________ >>>>>> infinispan-dev mailing list >>>>>> infinispan-dev(a)lists.jboss.org >>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>>>> >>>>> -- >>>>> Galder Zamarreño >>>>> Sr. Software Engineer >>>>> Infinispan, JBoss Cache >>>>> > _______________________________________________ >>>>> infinispan-dev mailing list >>>>> infinispan-dev(a)lists.jboss.org >>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>>> >>>> -- >>>> Manik Surtani >>>> manik(a)jboss.org >>>> Lead, Infinispan >>>> Lead, JBoss Cache >>>> http://www.infinispan.org >>>> http://www.jbosscache.org >>>> >>>> >>>> >>>> >>>> >>>> > _______________________________________________ >>>> infinispan-dev mailing list >>>> infinispan-dev(a)lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>> >>> -- >>> Galder Zamarreño >>> Sr. Software Engineer >>> Infinispan, JBoss Cache >>> _______________________________________________ >>> infinispan-dev mailing list >>> infinispan-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >> >> -- >> Manik Surtani >> manik(a)jboss.org >> Lead, Infinispan >> Lead, JBoss Cache >> http://www.infinispan.org >> http://www.jbosscache.org >> >> >> >> >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > -- > Galder Zamarreño > Sr. Software Engineer > Infinispan, JBoss Cache > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev > _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev