[jboss-jira] [JBoss JIRA] (JGRP-2433) Smaller changes in Message/BaseMessage

[Bela Ban (Jira)]

     [ https://issues.redhat.com/browse/JGRP-2433?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bela Ban updated JGRP-2433:
---------------------------
    Description: 
Based on Pedro's suggestions (copied from \[1\]):
*Comments/Suggestions*

I've created a new custom {{Message}} class to replace an ISPN command. It can be found here: https://github.com/pruivo/infinispan/blob/t_jgroups_5/core/src/main/java/org/infinispan/remoting/transport/jgroups/message/BackupAckMessage.java

* The set/get...Array/Object must have a default implementation in the interface. I doubt that any custom implementation will ever implement those methods. Similar idea for {{hasArray()}}

* Probably the {{size()}} shouldn't be implemented in the base class. Initially, I forgot to implement it :). A {{headersSize()}} method would be needed.
 
* It would be nice to have an abstract method write/read payload instead of overriding the writeTo/readFrom

* -{{copy()}} should be implemented in the base class and be final. Just add an abstract method {{copyPayload()}} that is invoked when the payload needs to be copied. The first parameter for {{copy()}} should have a better name, IMO.-

* -{{MessageFactory}} is kind of difficult to find. Can we have a method in the {{Channel}}? or {{getMessageFactory()}} or {{registerMessage(type, constructor)}}. Also, I found some protocol creating a new {{MessageFactory}}, they should use the same instance, right? And you allow to set a different  {{MessageFactory}} that isn't propagated everywhere... is there a use case where an user needs to replace the  {{MessageFactory}}? If not, just make it static somewhere-

* I'm wondering if there is way to avoid conflict to register a new message type. Imagine that wildfly and ispn creates a set new messages type, the conflict will only be detected at runtime. Also, the type is a byte... there is no much room to add custom messages types. 

* -Can the {{Flag}} and {{TransientFlag}} use the same short? There are only 2 transient flags so far and they can be the last bits of short. You can use a mask to skip sending them through the network-

* -no perf-ack numbers-

\[1\] https://issues.redhat.com/browse/JGRP-2218

  was:
Based on Pedro's suggestions (copied from \[1\]):
*Comments/Suggestions*

I've created a new custom {{Message}} class to replace an ISPN command. It can be found here: https://github.com/pruivo/infinispan/blob/t_jgroups_5/core/src/main/java/org/infinispan/remoting/transport/jgroups/message/BackupAckMessage.java

* The set/get...Array/Object must have a default implementation in the interface. I doubt that any custom implementation will ever implement those methods. Similar idea for {{hasArray()}}

* Probably the {{size()}} shouldn't be implemented in the base class. Initially, I forgot to implement it :). A {{headersSize()}} method would be needed.
 
* It would be nice to have an abstract method write/read payload instead of overriding the writeTo/readFrom

* {{copy()}} should be implemented in the base class and be final. Just add an abstract method {{copyPayload()}} that is invoked when the payload needs to be copied. The first parameter for {{copy()}} should have a better name, IMO.

* {{MessageFactory}} is kind of difficult to find. Can we have a method in the {{Channel}}? or {{getMessageFactory()}} or {{registerMessage(type, constructor)}}. Also, I found some protocol creating a new {{MessageFactory}}, they should use the same instance, right? And you allow to set a different  {{MessageFactory}} that isn't propagated everywhere... is there a use case where an user needs to replace the  {{MessageFactory}}? If not, just make it static somewhere :)

* I'm wondering if there is way to avoid conflict to register a new message type. Imagine that wildfly and ispn creates a set new messages type, the conflict will only be detected at runtime. Also, the type is a byte... there is no much room to add custom messages types. 

* Can the {{Flag}} and {{TransientFlag}} use the same short? There are only 2 transient flags so far and they can be the last bits of short. You can use a mask to skip sending them through the network 

* no perf-ack numbers :(

\[1\] https://issues.redhat.com/browse/JGRP-2218



> Smaller changes in Message/BaseMessage
> --------------------------------------
>
>                 Key: JGRP-2433
>                 URL: https://issues.redhat.com/browse/JGRP-2433
>             Project: JGroups
>          Issue Type: Task
>            Reporter: Bela Ban
>            Assignee: Bela Ban
>            Priority: Minor
>             Fix For: 5.0
>
>
> Based on Pedro's suggestions (copied from \[1\]):
> *Comments/Suggestions*
> I've created a new custom {{Message}} class to replace an ISPN command. It can be found here: https://github.com/pruivo/infinispan/blob/t_jgroups_5/core/src/main/java/org/infinispan/remoting/transport/jgroups/message/BackupAckMessage.java
> * The set/get...Array/Object must have a default implementation in the interface. I doubt that any custom implementation will ever implement those methods. Similar idea for {{hasArray()}}
> * Probably the {{size()}} shouldn't be implemented in the base class. Initially, I forgot to implement it :). A {{headersSize()}} method would be needed.
>  
> * It would be nice to have an abstract method write/read payload instead of overriding the writeTo/readFrom
> * -{{copy()}} should be implemented in the base class and be final. Just add an abstract method {{copyPayload()}} that is invoked when the payload needs to be copied. The first parameter for {{copy()}} should have a better name, IMO.-
> * -{{MessageFactory}} is kind of difficult to find. Can we have a method in the {{Channel}}? or {{getMessageFactory()}} or {{registerMessage(type, constructor)}}. Also, I found some protocol creating a new {{MessageFactory}}, they should use the same instance, right? And you allow to set a different  {{MessageFactory}} that isn't propagated everywhere... is there a use case where an user needs to replace the  {{MessageFactory}}? If not, just make it static somewhere-
> * I'm wondering if there is way to avoid conflict to register a new message type. Imagine that wildfly and ispn creates a set new messages type, the conflict will only be detected at runtime. Also, the type is a byte... there is no much room to add custom messages types. 
> * -Can the {{Flag}} and {{TransientFlag}} use the same short? There are only 2 transient flags so far and they can be the last bits of short. You can use a mask to skip sending them through the network-
> * -no perf-ack numbers-
> \[1\] https://issues.redhat.com/browse/JGRP-2218



--
This message was sent by Atlassian Jira
(v7.13.8#713008)