Committed by
Gerrit Code Review
Changed netty message type to String from Long to avoid potential collisions
Change-Id: I42014a920917a8022744ae15a9fefa6bae6890a7
Showing
5 changed files
with
34 additions
and
40 deletions
| ... | @@ -23,6 +23,7 @@ public enum DecoderState { | ... | @@ -23,6 +23,7 @@ public enum DecoderState { |
| 23 | READ_SENDER_IP_VERSION, | 23 | READ_SENDER_IP_VERSION, |
| 24 | READ_SENDER_IP, | 24 | READ_SENDER_IP, |
| 25 | READ_SENDER_PORT, | 25 | READ_SENDER_PORT, |
| 26 | + READ_MESSAGE_TYPE_LENGTH, | ||
| 26 | READ_MESSAGE_TYPE, | 27 | READ_MESSAGE_TYPE, |
| 27 | READ_CONTENT_LENGTH, | 28 | READ_CONTENT_LENGTH, |
| 28 | READ_CONTENT | 29 | READ_CONTENT | ... | ... |
| ... | @@ -27,19 +27,18 @@ import com.google.common.base.MoreObjects; | ... | @@ -27,19 +27,18 @@ import com.google.common.base.MoreObjects; |
| 27 | */ | 27 | */ |
| 28 | public final class InternalMessage implements Message { | 28 | public final class InternalMessage implements Message { |
| 29 | 29 | ||
| 30 | - public static final long REPLY_MESSAGE_TYPE = | 30 | + public static final String REPLY_MESSAGE_TYPE = "NETTY_MESSAGING_REQUEST_REPLY"; |
| 31 | - NettyMessagingService.hashToLong("NETTY_MESSAGING_REQUEST_REPLY"); | ||
| 32 | 31 | ||
| 33 | private long id; | 32 | private long id; |
| 34 | private Endpoint sender; | 33 | private Endpoint sender; |
| 35 | - private long type; | 34 | + private String type; |
| 36 | private byte[] payload; | 35 | private byte[] payload; |
| 37 | private transient NettyMessagingService messagingService; | 36 | private transient NettyMessagingService messagingService; |
| 38 | 37 | ||
| 39 | // Must be created using the Builder. | 38 | // Must be created using the Builder. |
| 40 | private InternalMessage() {} | 39 | private InternalMessage() {} |
| 41 | 40 | ||
| 42 | - InternalMessage(long id, Endpoint sender, long type, byte[] payload) { | 41 | + InternalMessage(long id, Endpoint sender, String type, byte[] payload) { |
| 43 | this.id = id; | 42 | this.id = id; |
| 44 | this.sender = sender; | 43 | this.sender = sender; |
| 45 | this.type = type; | 44 | this.type = type; |
| ... | @@ -50,7 +49,7 @@ public final class InternalMessage implements Message { | ... | @@ -50,7 +49,7 @@ public final class InternalMessage implements Message { |
| 50 | return id; | 49 | return id; |
| 51 | } | 50 | } |
| 52 | 51 | ||
| 53 | - public long type() { | 52 | + public String type() { |
| 54 | return type; | 53 | return type; |
| 55 | } | 54 | } |
| 56 | 55 | ||
| ... | @@ -104,7 +103,7 @@ public final class InternalMessage implements Message { | ... | @@ -104,7 +103,7 @@ public final class InternalMessage implements Message { |
| 104 | return this; | 103 | return this; |
| 105 | } | 104 | } |
| 106 | 105 | ||
| 107 | - public Builder withType(long type) { | 106 | + public Builder withType(String type) { |
| 108 | message.type = type; | 107 | message.type = type; |
| 109 | return this; | 108 | return this; |
| 110 | } | 109 | } | ... | ... |
| ... | @@ -27,6 +27,8 @@ import org.onlab.packet.IpAddress.Version; | ... | @@ -27,6 +27,8 @@ import org.onlab.packet.IpAddress.Version; |
| 27 | import org.slf4j.Logger; | 27 | import org.slf4j.Logger; |
| 28 | import org.slf4j.LoggerFactory; | 28 | import org.slf4j.LoggerFactory; |
| 29 | 29 | ||
| 30 | +import com.google.common.base.Charsets; | ||
| 31 | + | ||
| 30 | /** | 32 | /** |
| 31 | * Decoder for inbound messages. | 33 | * Decoder for inbound messages. |
| 32 | */ | 34 | */ |
| ... | @@ -40,8 +42,9 @@ public class MessageDecoder extends ReplayingDecoder<DecoderState> { | ... | @@ -40,8 +42,9 @@ public class MessageDecoder extends ReplayingDecoder<DecoderState> { |
| 40 | private Version ipVersion; | 42 | private Version ipVersion; |
| 41 | private IpAddress senderIp; | 43 | private IpAddress senderIp; |
| 42 | private int senderPort; | 44 | private int senderPort; |
| 45 | + private int messageTypeLength; | ||
| 46 | + private String messageType; | ||
| 43 | private int contentLength; | 47 | private int contentLength; |
| 44 | - private long messageType; | ||
| 45 | 48 | ||
| 46 | public MessageDecoder(NettyMessagingService messagingService) { | 49 | public MessageDecoder(NettyMessagingService messagingService) { |
| 47 | super(DecoderState.READ_MESSAGE_ID); | 50 | super(DecoderState.READ_MESSAGE_ID); |
| ... | @@ -68,9 +71,14 @@ public class MessageDecoder extends ReplayingDecoder<DecoderState> { | ... | @@ -68,9 +71,14 @@ public class MessageDecoder extends ReplayingDecoder<DecoderState> { |
| 68 | checkpoint(DecoderState.READ_SENDER_PORT); | 71 | checkpoint(DecoderState.READ_SENDER_PORT); |
| 69 | case READ_SENDER_PORT: | 72 | case READ_SENDER_PORT: |
| 70 | senderPort = buffer.readInt(); | 73 | senderPort = buffer.readInt(); |
| 74 | + checkpoint(DecoderState.READ_MESSAGE_TYPE_LENGTH); | ||
| 75 | + case READ_MESSAGE_TYPE_LENGTH: | ||
| 76 | + messageTypeLength = buffer.readInt(); | ||
| 71 | checkpoint(DecoderState.READ_MESSAGE_TYPE); | 77 | checkpoint(DecoderState.READ_MESSAGE_TYPE); |
| 72 | case READ_MESSAGE_TYPE: | 78 | case READ_MESSAGE_TYPE: |
| 73 | - messageType = buffer.readLong(); | 79 | + byte[] messageTypeBytes = new byte[messageTypeLength]; |
| 80 | + buffer.readBytes(messageTypeBytes); | ||
| 81 | + messageType = new String(messageTypeBytes, Charsets.UTF_8); | ||
| 74 | checkpoint(DecoderState.READ_CONTENT_LENGTH); | 82 | checkpoint(DecoderState.READ_CONTENT_LENGTH); |
| 75 | case READ_CONTENT_LENGTH: | 83 | case READ_CONTENT_LENGTH: |
| 76 | contentLength = buffer.readInt(); | 84 | contentLength = buffer.readInt(); | ... | ... |
| ... | @@ -27,6 +27,8 @@ import org.onlab.packet.IpAddress.Version; | ... | @@ -27,6 +27,8 @@ import org.onlab.packet.IpAddress.Version; |
| 27 | import org.slf4j.Logger; | 27 | import org.slf4j.Logger; |
| 28 | import org.slf4j.LoggerFactory; | 28 | import org.slf4j.LoggerFactory; |
| 29 | 29 | ||
| 30 | +import com.google.common.base.Charsets; | ||
| 31 | + | ||
| 30 | /** | 32 | /** |
| 31 | * Encode InternalMessage out into a byte buffer. | 33 | * Encode InternalMessage out into a byte buffer. |
| 32 | */ | 34 | */ |
| ... | @@ -57,8 +59,13 @@ public class MessageEncoder extends MessageToByteEncoder<InternalMessage> { | ... | @@ -57,8 +59,13 @@ public class MessageEncoder extends MessageToByteEncoder<InternalMessage> { |
| 57 | // write sender port | 59 | // write sender port |
| 58 | out.writeInt(sender.port()); | 60 | out.writeInt(sender.port()); |
| 59 | 61 | ||
| 60 | - // write message type. | 62 | + byte[] messageTypeBytes = message.type().getBytes(Charsets.UTF_8); |
| 61 | - out.writeLong(message.type()); | 63 | + |
| 64 | + // write length of message type | ||
| 65 | + out.writeInt(messageTypeBytes.length); | ||
| 66 | + | ||
| 67 | + // write message type bytes | ||
| 68 | + out.writeBytes(messageTypeBytes); | ||
| 62 | 69 | ||
| 63 | byte[] payload = message.payload(); | 70 | byte[] payload = message.payload(); |
| 64 | 71 | ... | ... |
| ... | @@ -52,14 +52,10 @@ import org.onlab.packet.IpAddress; | ... | @@ -52,14 +52,10 @@ import org.onlab.packet.IpAddress; |
| 52 | import org.slf4j.Logger; | 52 | import org.slf4j.Logger; |
| 53 | import org.slf4j.LoggerFactory; | 53 | import org.slf4j.LoggerFactory; |
| 54 | 54 | ||
| 55 | -import com.google.common.base.Charsets; | ||
| 56 | import com.google.common.cache.Cache; | 55 | import com.google.common.cache.Cache; |
| 57 | import com.google.common.cache.CacheBuilder; | 56 | import com.google.common.cache.CacheBuilder; |
| 58 | -import com.google.common.cache.CacheLoader; | ||
| 59 | -import com.google.common.cache.LoadingCache; | ||
| 60 | import com.google.common.cache.RemovalListener; | 57 | import com.google.common.cache.RemovalListener; |
| 61 | import com.google.common.cache.RemovalNotification; | 58 | import com.google.common.cache.RemovalNotification; |
| 62 | -import com.google.common.hash.Hashing; | ||
| 63 | import com.google.common.util.concurrent.ListenableFuture; | 59 | import com.google.common.util.concurrent.ListenableFuture; |
| 64 | import com.google.common.util.concurrent.SettableFuture; | 60 | import com.google.common.util.concurrent.SettableFuture; |
| 65 | 61 | ||
| ... | @@ -71,7 +67,7 @@ public class NettyMessagingService implements MessagingService { | ... | @@ -71,7 +67,7 @@ public class NettyMessagingService implements MessagingService { |
| 71 | private final Logger log = LoggerFactory.getLogger(getClass()); | 67 | private final Logger log = LoggerFactory.getLogger(getClass()); |
| 72 | 68 | ||
| 73 | private final Endpoint localEp; | 69 | private final Endpoint localEp; |
| 74 | - private final ConcurrentMap<Long, MessageHandler> handlers = new ConcurrentHashMap<>(); | 70 | + private final ConcurrentMap<String, MessageHandler> handlers = new ConcurrentHashMap<>(); |
| 75 | private final AtomicLong messageIdGenerator = new AtomicLong(0); | 71 | private final AtomicLong messageIdGenerator = new AtomicLong(0); |
| 76 | private final Cache<Long, SettableFuture<byte[]>> responseFutures = CacheBuilder.newBuilder() | 72 | private final Cache<Long, SettableFuture<byte[]>> responseFutures = CacheBuilder.newBuilder() |
| 77 | .maximumSize(100000) | 73 | .maximumSize(100000) |
| ... | @@ -86,14 +82,6 @@ public class NettyMessagingService implements MessagingService { | ... | @@ -86,14 +82,6 @@ public class NettyMessagingService implements MessagingService { |
| 86 | }) | 82 | }) |
| 87 | .build(); | 83 | .build(); |
| 88 | 84 | ||
| 89 | - private final LoadingCache<String, Long> messageTypeLookupCache = CacheBuilder.newBuilder() | ||
| 90 | - .build(new CacheLoader<String, Long>() { | ||
| 91 | - @Override | ||
| 92 | - public Long load(String type) { | ||
| 93 | - return hashToLong(type); | ||
| 94 | - } | ||
| 95 | - }); | ||
| 96 | - | ||
| 97 | private final GenericKeyedObjectPool<Endpoint, Channel> channels | 85 | private final GenericKeyedObjectPool<Endpoint, Channel> channels |
| 98 | = new GenericKeyedObjectPool<Endpoint, Channel>(new OnosCommunicationChannelFactory()); | 86 | = new GenericKeyedObjectPool<Endpoint, Channel>(new OnosCommunicationChannelFactory()); |
| 99 | 87 | ||
| ... | @@ -162,7 +150,7 @@ public class NettyMessagingService implements MessagingService { | ... | @@ -162,7 +150,7 @@ public class NettyMessagingService implements MessagingService { |
| 162 | InternalMessage message = new InternalMessage.Builder(this) | 150 | InternalMessage message = new InternalMessage.Builder(this) |
| 163 | .withId(messageIdGenerator.incrementAndGet()) | 151 | .withId(messageIdGenerator.incrementAndGet()) |
| 164 | .withSender(localEp) | 152 | .withSender(localEp) |
| 165 | - .withType(messageTypeLookupCache.getUnchecked(type)) | 153 | + .withType(type) |
| 166 | .withPayload(payload) | 154 | .withPayload(payload) |
| 167 | .build(); | 155 | .build(); |
| 168 | sendAsync(ep, message); | 156 | sendAsync(ep, message); |
| ... | @@ -198,7 +186,7 @@ public class NettyMessagingService implements MessagingService { | ... | @@ -198,7 +186,7 @@ public class NettyMessagingService implements MessagingService { |
| 198 | InternalMessage message = new InternalMessage.Builder(this) | 186 | InternalMessage message = new InternalMessage.Builder(this) |
| 199 | .withId(messageId) | 187 | .withId(messageId) |
| 200 | .withSender(localEp) | 188 | .withSender(localEp) |
| 201 | - .withType(messageTypeLookupCache.getUnchecked(type)) | 189 | + .withType(type) |
| 202 | .withPayload(payload) | 190 | .withPayload(payload) |
| 203 | .build(); | 191 | .build(); |
| 204 | try { | 192 | try { |
| ... | @@ -212,12 +200,12 @@ public class NettyMessagingService implements MessagingService { | ... | @@ -212,12 +200,12 @@ public class NettyMessagingService implements MessagingService { |
| 212 | 200 | ||
| 213 | @Override | 201 | @Override |
| 214 | public void registerHandler(String type, MessageHandler handler) { | 202 | public void registerHandler(String type, MessageHandler handler) { |
| 215 | - handlers.putIfAbsent(hashToLong(type), handler); | 203 | + handlers.putIfAbsent(type, handler); |
| 216 | } | 204 | } |
| 217 | 205 | ||
| 218 | @Override | 206 | @Override |
| 219 | public void registerHandler(String type, MessageHandler handler, ExecutorService executor) { | 207 | public void registerHandler(String type, MessageHandler handler, ExecutorService executor) { |
| 220 | - handlers.putIfAbsent(hashToLong(type), new MessageHandler() { | 208 | + handlers.putIfAbsent(type, new MessageHandler() { |
| 221 | @Override | 209 | @Override |
| 222 | public void handle(Message message) throws IOException { | 210 | public void handle(Message message) throws IOException { |
| 223 | executor.submit(() -> { | 211 | executor.submit(() -> { |
| ... | @@ -233,10 +221,10 @@ public class NettyMessagingService implements MessagingService { | ... | @@ -233,10 +221,10 @@ public class NettyMessagingService implements MessagingService { |
| 233 | 221 | ||
| 234 | @Override | 222 | @Override |
| 235 | public void unregisterHandler(String type) { | 223 | public void unregisterHandler(String type) { |
| 236 | - handlers.remove(hashToLong(type)); | 224 | + handlers.remove(type); |
| 237 | } | 225 | } |
| 238 | 226 | ||
| 239 | - private MessageHandler getMessageHandler(long type) { | 227 | + private MessageHandler getMessageHandler(String type) { |
| 240 | return handlers.get(type); | 228 | return handlers.get(type); |
| 241 | } | 229 | } |
| 242 | 230 | ||
| ... | @@ -342,8 +330,8 @@ public class NettyMessagingService implements MessagingService { | ... | @@ -342,8 +330,8 @@ public class NettyMessagingService implements MessagingService { |
| 342 | } | 330 | } |
| 343 | 331 | ||
| 344 | private void dispatchLocally(InternalMessage message) throws IOException { | 332 | private void dispatchLocally(InternalMessage message) throws IOException { |
| 345 | - long type = message.type(); | 333 | + String type = message.type(); |
| 346 | - if (type == InternalMessage.REPLY_MESSAGE_TYPE) { | 334 | + if (InternalMessage.REPLY_MESSAGE_TYPE.equals(type)) { |
| 347 | try { | 335 | try { |
| 348 | SettableFuture<byte[]> futureResponse = | 336 | SettableFuture<byte[]> futureResponse = |
| 349 | NettyMessagingService.this.responseFutures.getIfPresent(message.id()); | 337 | NettyMessagingService.this.responseFutures.getIfPresent(message.id()); |
| ... | @@ -366,13 +354,4 @@ public class NettyMessagingService implements MessagingService { | ... | @@ -366,13 +354,4 @@ public class NettyMessagingService implements MessagingService { |
| 366 | log.debug("No handler registered for {}", type); | 354 | log.debug("No handler registered for {}", type); |
| 367 | } | 355 | } |
| 368 | } | 356 | } |
| 369 | - | ||
| 370 | - /** | ||
| 371 | - * Returns the md5 hash of the specified input string as a long. | ||
| 372 | - * @param input input string. | ||
| 373 | - * @return md5 hash as long. | ||
| 374 | - */ | ||
| 375 | - public static long hashToLong(String input) { | ||
| 376 | - return Hashing.md5().hashBytes(input.getBytes(Charsets.UTF_8)).asLong(); | ||
| 377 | - } | ||
| 378 | } | 357 | } |
| ... | \ No newline at end of file | ... | \ No newline at end of file | ... | ... |
-
Please register or login to post a comment