• Icon: Task Task
    • Resolution: Done
    • Icon: Minor Minor
    • 5.0.0.Alpha3
    • None
    • None

      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

              rhn-engineering-bban Bela Ban
              rhn-engineering-bban Bela Ban
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Created:
                Updated:
                Resolved: