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 <galder(a)redhat.com> wrote:
From: Galder Zamarreno <galder(a)redhat.com>
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