Re: [infinispan-dev] Hot Rod - pt2

> When I implemented a similar protocol I kept > the response code. I suppose you mean that you kept the opt code?

Why would you want the error to be parsed by a different decoder?

> I haven't thought too deeply on the client implementation itself...

p.s. Thanks for the feedback :)

From: Galder Zamarreno <galder(a)redhat.com&gt; Subject: Re: [infinispan-dev] Hot Rod - pt2 To: infinispan-dev(a)lists.jboss.org Date: Monday, January 4, 2010, 5:25 AM On 12/29/2009 07:35 PM, Alex Kluge wrote: >> 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. I suppose you mean that you kept the opt 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. Why would you want the error to be parsed by a different decoder? > > I also made it easy to register custom requests and responses, and this > is facilitated by having distinct op codes for each. True. If you have a custom op in a custom request and response, this can be handled by a custom message decoder, assuming that the granularity of the message decoder would be based on the op code. This would indeed make it easier for the protocol to be extended. I haven't thought too deeply on the client implementation itself, so this will be clearer once I get around to doing that. For the time being, I'll leave it as it is and will revisit this decision at that point. > >                                                    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 -- 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