Committed by
Gerrit Code Review
Bypass netty stack for messages that are sent to self
Change-Id: Ifb1fd610892bd22a291cda472a8a5ef7a1dcfe6d Manual serde for ClusterMessage to avoid one additional kryo serialization overhead for each message sent/received Change-Id: I08d9a2c10403b0e9e9e1736c6bd36fa008bb8db0
Showing
3 changed files
with
65 additions
and
37 deletions
... | @@ -15,14 +15,17 @@ | ... | @@ -15,14 +15,17 @@ |
15 | */ | 15 | */ |
16 | package org.onosproject.store.cluster.messaging; | 16 | package org.onosproject.store.cluster.messaging; |
17 | 17 | ||
18 | -import com.google.common.base.MoreObjects; | ||
19 | -import org.onlab.util.ByteArraySizeHashPrinter; | ||
20 | -import org.onosproject.cluster.NodeId; | ||
21 | - | ||
22 | import java.io.IOException; | 18 | import java.io.IOException; |
19 | +import java.nio.ByteBuffer; | ||
23 | import java.util.Arrays; | 20 | import java.util.Arrays; |
24 | import java.util.Objects; | 21 | import java.util.Objects; |
25 | 22 | ||
23 | +import org.onlab.util.ByteArraySizeHashPrinter; | ||
24 | +import org.onosproject.cluster.NodeId; | ||
25 | + | ||
26 | +import com.google.common.base.Charsets; | ||
27 | +import com.google.common.base.MoreObjects; | ||
28 | + | ||
26 | // TODO: Should payload type be ByteBuffer? | 29 | // TODO: Should payload type be ByteBuffer? |
27 | /** | 30 | /** |
28 | * Base message for cluster-wide communications. | 31 | * Base message for cluster-wide communications. |
... | @@ -105,6 +108,43 @@ public class ClusterMessage { | ... | @@ -105,6 +108,43 @@ public class ClusterMessage { |
105 | Arrays.equals(this.payload, that.payload); | 108 | Arrays.equals(this.payload, that.payload); |
106 | } | 109 | } |
107 | 110 | ||
111 | + /** | ||
112 | + * Serializes this instance. | ||
113 | + * @return bytes | ||
114 | + */ | ||
115 | + public byte[] getBytes() { | ||
116 | + byte[] senderBytes = sender.toString().getBytes(Charsets.UTF_8); | ||
117 | + byte[] subjectBytes = subject.value().getBytes(Charsets.UTF_8); | ||
118 | + int capacity = 12 + senderBytes.length + subjectBytes.length + payload.length; | ||
119 | + ByteBuffer buffer = ByteBuffer.allocate(capacity); | ||
120 | + buffer.putInt(senderBytes.length); | ||
121 | + buffer.put(senderBytes); | ||
122 | + buffer.putInt(subjectBytes.length); | ||
123 | + buffer.put(subjectBytes); | ||
124 | + buffer.putInt(payload.length); | ||
125 | + buffer.put(payload); | ||
126 | + return buffer.array(); | ||
127 | + } | ||
128 | + | ||
129 | + /** | ||
130 | + * Decodes a new ClusterMessage from raw bytes. | ||
131 | + * @param bytes raw bytes | ||
132 | + * @return cluster message | ||
133 | + */ | ||
134 | + public static ClusterMessage fromBytes(byte[] bytes) { | ||
135 | + ByteBuffer buffer = ByteBuffer.wrap(bytes); | ||
136 | + byte[] senderBytes = new byte[buffer.getInt()]; | ||
137 | + buffer.get(senderBytes); | ||
138 | + byte[] subjectBytes = new byte[buffer.getInt()]; | ||
139 | + buffer.get(subjectBytes); | ||
140 | + byte[] payloadBytes = new byte[buffer.getInt()]; | ||
141 | + buffer.get(payloadBytes); | ||
142 | + | ||
143 | + return new ClusterMessage(new NodeId(new String(senderBytes, Charsets.UTF_8)), | ||
144 | + new MessageSubject(new String(senderBytes, Charsets.UTF_8)), | ||
145 | + payloadBytes); | ||
146 | + } | ||
147 | + | ||
108 | @Override | 148 | @Override |
109 | public int hashCode() { | 149 | public int hashCode() { |
110 | return Objects.hash(sender, subject, payload); | 150 | return Objects.hash(sender, subject, payload); | ... | ... |
... | @@ -28,7 +28,6 @@ import org.onlab.netty.Message; | ... | @@ -28,7 +28,6 @@ import org.onlab.netty.Message; |
28 | import org.onlab.netty.MessageHandler; | 28 | import org.onlab.netty.MessageHandler; |
29 | import org.onlab.netty.MessagingService; | 29 | import org.onlab.netty.MessagingService; |
30 | import org.onlab.netty.NettyMessagingService; | 30 | import org.onlab.netty.NettyMessagingService; |
31 | -import org.onlab.util.KryoNamespace; | ||
32 | import org.onosproject.cluster.ClusterService; | 31 | import org.onosproject.cluster.ClusterService; |
33 | import org.onosproject.cluster.ControllerNode; | 32 | import org.onosproject.cluster.ControllerNode; |
34 | import org.onosproject.cluster.NodeId; | 33 | import org.onosproject.cluster.NodeId; |
... | @@ -36,10 +35,6 @@ import org.onosproject.store.cluster.messaging.ClusterCommunicationService; | ... | @@ -36,10 +35,6 @@ import org.onosproject.store.cluster.messaging.ClusterCommunicationService; |
36 | import org.onosproject.store.cluster.messaging.ClusterMessage; | 35 | import org.onosproject.store.cluster.messaging.ClusterMessage; |
37 | import org.onosproject.store.cluster.messaging.ClusterMessageHandler; | 36 | import org.onosproject.store.cluster.messaging.ClusterMessageHandler; |
38 | import org.onosproject.store.cluster.messaging.MessageSubject; | 37 | import org.onosproject.store.cluster.messaging.MessageSubject; |
39 | -import org.onosproject.store.serializers.KryoNamespaces; | ||
40 | -import org.onosproject.store.serializers.KryoSerializer; | ||
41 | -import org.onosproject.store.serializers.impl.ClusterMessageSerializer; | ||
42 | -import org.onosproject.store.serializers.impl.MessageSubjectSerializer; | ||
43 | import org.slf4j.Logger; | 38 | import org.slf4j.Logger; |
44 | import org.slf4j.LoggerFactory; | 39 | import org.slf4j.LoggerFactory; |
45 | 40 | ||
... | @@ -62,19 +57,6 @@ public class ClusterCommunicationManager | ... | @@ -62,19 +57,6 @@ public class ClusterCommunicationManager |
62 | // TODO: This probably should not be a OSGi service. | 57 | // TODO: This probably should not be a OSGi service. |
63 | private MessagingService messagingService; | 58 | private MessagingService messagingService; |
64 | 59 | ||
65 | - private static final KryoSerializer SERIALIZER = new KryoSerializer() { | ||
66 | - @Override | ||
67 | - protected void setupKryoPool() { | ||
68 | - serializerPool = KryoNamespace.newBuilder() | ||
69 | - .register(KryoNamespaces.API) | ||
70 | - .nextId(KryoNamespaces.BEGIN_USER_CUSTOM_ID) | ||
71 | - .register(new ClusterMessageSerializer(), ClusterMessage.class) | ||
72 | - .register(new MessageSubjectSerializer(), MessageSubject.class) | ||
73 | - .build(); | ||
74 | - } | ||
75 | - | ||
76 | - }; | ||
77 | - | ||
78 | @Activate | 60 | @Activate |
79 | public void activate() { | 61 | public void activate() { |
80 | ControllerNode localNode = clusterService.getLocalNode(); | 62 | ControllerNode localNode = clusterService.getLocalNode(); |
... | @@ -105,7 +87,7 @@ public class ClusterCommunicationManager | ... | @@ -105,7 +87,7 @@ public class ClusterCommunicationManager |
105 | public boolean broadcast(ClusterMessage message) { | 87 | public boolean broadcast(ClusterMessage message) { |
106 | boolean ok = true; | 88 | boolean ok = true; |
107 | final ControllerNode localNode = clusterService.getLocalNode(); | 89 | final ControllerNode localNode = clusterService.getLocalNode(); |
108 | - byte[] payload = SERIALIZER.encode(message); | 90 | + byte[] payload = message.getBytes(); |
109 | for (ControllerNode node : clusterService.getNodes()) { | 91 | for (ControllerNode node : clusterService.getNodes()) { |
110 | if (!node.equals(localNode)) { | 92 | if (!node.equals(localNode)) { |
111 | ok = unicastUnchecked(message.subject(), payload, node.id()) && ok; | 93 | ok = unicastUnchecked(message.subject(), payload, node.id()) && ok; |
... | @@ -117,7 +99,7 @@ public class ClusterCommunicationManager | ... | @@ -117,7 +99,7 @@ public class ClusterCommunicationManager |
117 | @Override | 99 | @Override |
118 | public boolean broadcastIncludeSelf(ClusterMessage message) { | 100 | public boolean broadcastIncludeSelf(ClusterMessage message) { |
119 | boolean ok = true; | 101 | boolean ok = true; |
120 | - byte[] payload = SERIALIZER.encode(message); | 102 | + byte[] payload = message.getBytes(); |
121 | for (ControllerNode node : clusterService.getNodes()) { | 103 | for (ControllerNode node : clusterService.getNodes()) { |
122 | ok = unicastUnchecked(message.subject(), payload, node.id()) && ok; | 104 | ok = unicastUnchecked(message.subject(), payload, node.id()) && ok; |
123 | } | 105 | } |
... | @@ -128,7 +110,7 @@ public class ClusterCommunicationManager | ... | @@ -128,7 +110,7 @@ public class ClusterCommunicationManager |
128 | public boolean multicast(ClusterMessage message, Set<NodeId> nodes) { | 110 | public boolean multicast(ClusterMessage message, Set<NodeId> nodes) { |
129 | boolean ok = true; | 111 | boolean ok = true; |
130 | final ControllerNode localNode = clusterService.getLocalNode(); | 112 | final ControllerNode localNode = clusterService.getLocalNode(); |
131 | - byte[] payload = SERIALIZER.encode(message); | 113 | + byte[] payload = message.getBytes(); |
132 | for (NodeId nodeId : nodes) { | 114 | for (NodeId nodeId : nodes) { |
133 | if (!nodeId.equals(localNode.id())) { | 115 | if (!nodeId.equals(localNode.id())) { |
134 | ok = unicastUnchecked(message.subject(), payload, nodeId) && ok; | 116 | ok = unicastUnchecked(message.subject(), payload, nodeId) && ok; |
... | @@ -139,7 +121,7 @@ public class ClusterCommunicationManager | ... | @@ -139,7 +121,7 @@ public class ClusterCommunicationManager |
139 | 121 | ||
140 | @Override | 122 | @Override |
141 | public boolean unicast(ClusterMessage message, NodeId toNodeId) throws IOException { | 123 | public boolean unicast(ClusterMessage message, NodeId toNodeId) throws IOException { |
142 | - return unicast(message.subject(), SERIALIZER.encode(message), toNodeId); | 124 | + return unicast(message.subject(), message.getBytes(), toNodeId); |
143 | } | 125 | } |
144 | 126 | ||
145 | private boolean unicast(MessageSubject subject, byte[] payload, NodeId toNodeId) throws IOException { | 127 | private boolean unicast(MessageSubject subject, byte[] payload, NodeId toNodeId) throws IOException { |
... | @@ -170,7 +152,7 @@ public class ClusterCommunicationManager | ... | @@ -170,7 +152,7 @@ public class ClusterCommunicationManager |
170 | checkArgument(node != null, "Unknown nodeId: %s", toNodeId); | 152 | checkArgument(node != null, "Unknown nodeId: %s", toNodeId); |
171 | Endpoint nodeEp = new Endpoint(node.ip(), node.tcpPort()); | 153 | Endpoint nodeEp = new Endpoint(node.ip(), node.tcpPort()); |
172 | try { | 154 | try { |
173 | - return messagingService.sendAndReceive(nodeEp, message.subject().value(), SERIALIZER.encode(message)); | 155 | + return messagingService.sendAndReceive(nodeEp, message.subject().value(), message.getBytes()); |
174 | 156 | ||
175 | } catch (IOException e) { | 157 | } catch (IOException e) { |
176 | log.trace("Failed interaction with remote nodeId: " + toNodeId, e); | 158 | log.trace("Failed interaction with remote nodeId: " + toNodeId, e); |
... | @@ -209,7 +191,7 @@ public class ClusterCommunicationManager | ... | @@ -209,7 +191,7 @@ public class ClusterCommunicationManager |
209 | public void handle(Message message) { | 191 | public void handle(Message message) { |
210 | final ClusterMessage clusterMessage; | 192 | final ClusterMessage clusterMessage; |
211 | try { | 193 | try { |
212 | - clusterMessage = SERIALIZER.decode(message.payload()); | 194 | + clusterMessage = ClusterMessage.fromBytes(message.payload()); |
213 | } catch (Exception e) { | 195 | } catch (Exception e) { |
214 | log.error("Failed decoding {}", message, e); | 196 | log.error("Failed decoding {}", message, e); |
215 | throw e; | 197 | throw e; | ... | ... |
... | @@ -87,9 +87,7 @@ public class NettyMessagingService implements MessagingService { | ... | @@ -87,9 +87,7 @@ public class NettyMessagingService implements MessagingService { |
87 | .build(); | 87 | .build(); |
88 | 88 | ||
89 | private final LoadingCache<String, Long> messageTypeLookupCache = CacheBuilder.newBuilder() | 89 | private final LoadingCache<String, Long> messageTypeLookupCache = CacheBuilder.newBuilder() |
90 | - .softValues() | ||
91 | .build(new CacheLoader<String, Long>() { | 90 | .build(new CacheLoader<String, Long>() { |
92 | - | ||
93 | @Override | 91 | @Override |
94 | public Long load(String type) { | 92 | public Long load(String type) { |
95 | return hashToLong(type); | 93 | return hashToLong(type); |
... | @@ -171,6 +169,10 @@ public class NettyMessagingService implements MessagingService { | ... | @@ -171,6 +169,10 @@ public class NettyMessagingService implements MessagingService { |
171 | } | 169 | } |
172 | 170 | ||
173 | protected void sendAsync(Endpoint ep, InternalMessage message) throws IOException { | 171 | protected void sendAsync(Endpoint ep, InternalMessage message) throws IOException { |
172 | + if (ep.equals(localEp)) { | ||
173 | + dispatchLocally(message); | ||
174 | + return; | ||
175 | + } | ||
174 | Channel channel = null; | 176 | Channel channel = null; |
175 | try { | 177 | try { |
176 | try { | 178 | try { |
... | @@ -329,6 +331,17 @@ public class NettyMessagingService implements MessagingService { | ... | @@ -329,6 +331,17 @@ public class NettyMessagingService implements MessagingService { |
329 | 331 | ||
330 | @Override | 332 | @Override |
331 | protected void channelRead0(ChannelHandlerContext ctx, InternalMessage message) throws Exception { | 333 | protected void channelRead0(ChannelHandlerContext ctx, InternalMessage message) throws Exception { |
334 | + dispatchLocally(message); | ||
335 | + } | ||
336 | + | ||
337 | + @Override | ||
338 | + public void exceptionCaught(ChannelHandlerContext context, Throwable cause) { | ||
339 | + log.error("Exception inside channel handling pipeline.", cause); | ||
340 | + context.close(); | ||
341 | + } | ||
342 | + } | ||
343 | + | ||
344 | + private void dispatchLocally(InternalMessage message) throws IOException { | ||
332 | long type = message.type(); | 345 | long type = message.type(); |
333 | if (type == InternalMessage.REPLY_MESSAGE_TYPE) { | 346 | if (type == InternalMessage.REPLY_MESSAGE_TYPE) { |
334 | try { | 347 | try { |
... | @@ -354,13 +367,6 @@ public class NettyMessagingService implements MessagingService { | ... | @@ -354,13 +367,6 @@ public class NettyMessagingService implements MessagingService { |
354 | } | 367 | } |
355 | } | 368 | } |
356 | 369 | ||
357 | - @Override | ||
358 | - public void exceptionCaught(ChannelHandlerContext context, Throwable cause) { | ||
359 | - log.error("Exception inside channel handling pipeline.", cause); | ||
360 | - context.close(); | ||
361 | - } | ||
362 | - } | ||
363 | - | ||
364 | /** | 370 | /** |
365 | * Returns the md5 hash of the specified input string as a long. | 371 | * Returns the md5 hash of the specified input string as a long. |
366 | * @param input input string. | 372 | * @param input input string. | ... | ... |
-
Please register or login to post a comment