[infinispan-dev] Topology Id in Hotrod header

[Mircea Markus]

On 16 Jul 2010, at 10:43, Galder Zamarreño wrote:

> 
> On Jul 16, 2010, at 11:10 AM, Mircea Markus wrote:
> 
>> 
>> On 16 Jul 2010, at 09:11, Galder Zamarreño wrote:
>> 
>>> 
>>> On Jul 15, 2010, at 7:08 PM, Mircea Markus wrote:
>>> 
>>>> Hi,
>>>> 
>>>> The header for a request is composed of: 
>>>> 
>>>> Magic	Message Id	Version	Opcode	Cache Name Length	Cache Name	Flags	Client Intelligence	Topology Id
>>>> 
>>>> 
>>>> We send "Topology Id " even if the client is not topology aware. Wouldn't it be more effective to only expect it if the client's intelligence is 02 or 03 and not send it otherwise?
>>> 
>>> I think doing things like this starts to complicate protocol parsing.
>> If I'm not missing something, this would be solved through a single if statement:
>> topologyId = 0;
>> if (intelligence == 3) {
>> topologyId = stream.readVint();
>> }
>> This increases performance as we don't send data that is not needed. I'm about to add transaction Id to operations as well, and I'd rather not add it if it is not needed. 
> 
> You forgot to add an OR intelligence == 2.
> 
> It's not the complication of adding that if, it's the complication of adding if statements like that all around if some fields might or might not appear. Also, it's not just the server that needs to code this but all client impls out there. So far it's only one client though.
Intelligence level on clients is not configurable (should it be?) - so there's no "if" on the client side.
I don't see a lot of complication to support this, really. 
> 
> For transaction id, you want it to be optional too? Based on what rule would you read or not read the transaction ID? 
I reckon I'll have to do it optional: e.g. a transactional put would carry the transaction id with it, but not in the header(there are operations that are not transactionable, e.g. ping, maybe even versioned operations so no point for these to carry the tx id in header as well).
so put request, in case of transactionable clients, would be postfixed with tx id.
> 
> Are you planning to add transaction id or make this change for 4.1 or 5.0? If you're doing it for 4.1, I'd just add it to the header as mandatory. If you're doing it for 5.0, that'd be a different protocol version and we can maybe try to experiment there and we could compare with 4.1 perf afterwards.
for 5 
> 
>>> Having optional fields that are present or not depending of contents is complicating things, particularly when we're talking about the header which is common to everyone.
>>> 
>>>> 
>>>> Cheers,
>>>> Mircea   
>>>> _______________________________________________
>>>> infinispan-dev mailing list
>>>> infinispan-dev at 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 at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> 
>> 
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev at 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 at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev