Committed by
Gerrit Code Review
[Falcon] Adds a status field to InternalMessage and support for replying with ap…
…propriate status when handler errors occur Change-Id: I995bdd6c67b88b6d7729887d32083315213fb79f
Showing
6 changed files
with
146 additions
and
30 deletions
1 | +/* | ||
2 | + * Copyright 2016 Open Networking Laboratory | ||
3 | + * | ||
4 | + * Licensed under the Apache License, Version 2.0 (the "License"); | ||
5 | + * you may not use this file except in compliance with the License. | ||
6 | + * You may obtain a copy of the License at | ||
7 | + * | ||
8 | + * http://www.apache.org/licenses/LICENSE-2.0 | ||
9 | + * | ||
10 | + * Unless required by applicable law or agreed to in writing, software | ||
11 | + * distributed under the License is distributed on an "AS IS" BASIS, | ||
12 | + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
13 | + * See the License for the specific language governing permissions and | ||
14 | + * limitations under the License. | ||
15 | + */ | ||
16 | +package org.onosproject.store.cluster.messaging; | ||
17 | + | ||
18 | +import java.io.IOException; | ||
19 | + | ||
20 | +/** | ||
21 | + * Top level exception for MessagingService failures. | ||
22 | + */ | ||
23 | +@SuppressWarnings("serial") | ||
24 | +public class MessagingException extends IOException { | ||
25 | + | ||
26 | + public MessagingException() { | ||
27 | + } | ||
28 | + | ||
29 | + public MessagingException(String message) { | ||
30 | + super(message); | ||
31 | + } | ||
32 | + | ||
33 | + public MessagingException(String message, Throwable t) { | ||
34 | + super(message, t); | ||
35 | + } | ||
36 | + | ||
37 | + public MessagingException(Throwable t) { | ||
38 | + super(t); | ||
39 | + } | ||
40 | + | ||
41 | + /** | ||
42 | + * Exception indicating no remote registered remote handler. | ||
43 | + */ | ||
44 | + public static class NoRemoteHandler extends MessagingException { | ||
45 | + } | ||
46 | + | ||
47 | + /** | ||
48 | + * Exception indicating handler failure. | ||
49 | + */ | ||
50 | + public static class RemoteHandlerFailure extends MessagingException { | ||
51 | + } | ||
52 | +} | ||
... | \ No newline at end of file | ... | \ No newline at end of file |
... | @@ -26,6 +26,7 @@ public enum DecoderState { | ... | @@ -26,6 +26,7 @@ public enum DecoderState { |
26 | READ_SENDER_PORT, | 26 | READ_SENDER_PORT, |
27 | READ_MESSAGE_TYPE_LENGTH, | 27 | READ_MESSAGE_TYPE_LENGTH, |
28 | READ_MESSAGE_TYPE, | 28 | READ_MESSAGE_TYPE, |
29 | + READ_MESSAGE_STATUS, | ||
29 | READ_CONTENT_LENGTH, | 30 | READ_CONTENT_LENGTH, |
30 | READ_CONTENT | 31 | READ_CONTENT |
31 | } | 32 | } | ... | ... |
... | @@ -25,16 +25,46 @@ import org.onosproject.store.cluster.messaging.Endpoint; | ... | @@ -25,16 +25,46 @@ import org.onosproject.store.cluster.messaging.Endpoint; |
25 | */ | 25 | */ |
26 | public final class InternalMessage { | 26 | public final class InternalMessage { |
27 | 27 | ||
28 | + /** | ||
29 | + * Message status. | ||
30 | + */ | ||
31 | + public enum Status { | ||
32 | + /** | ||
33 | + * All ok. | ||
34 | + */ | ||
35 | + OK, | ||
36 | + | ||
37 | + /** | ||
38 | + * Response status signifying no registered handler. | ||
39 | + */ | ||
40 | + ERROR_NO_HANDLER, | ||
41 | + | ||
42 | + /** | ||
43 | + * Response status signifying an exception handling the message. | ||
44 | + */ | ||
45 | + ERROR_HANDLER_EXCEPTION | ||
46 | + | ||
47 | + // NOTE: For backwards compatibility it important that new enum constants | ||
48 | + // be appended. | ||
49 | + // FIXME: We should remove this restriction in the future. | ||
50 | + } | ||
51 | + | ||
28 | private final long id; | 52 | private final long id; |
29 | private final Endpoint sender; | 53 | private final Endpoint sender; |
30 | private final String type; | 54 | private final String type; |
31 | private final byte[] payload; | 55 | private final byte[] payload; |
56 | + private final Status status; | ||
32 | 57 | ||
33 | public InternalMessage(long id, Endpoint sender, String type, byte[] payload) { | 58 | public InternalMessage(long id, Endpoint sender, String type, byte[] payload) { |
59 | + this(id, sender, type, payload, Status.OK); | ||
60 | + } | ||
61 | + | ||
62 | + public InternalMessage(long id, Endpoint sender, String type, byte[] payload, Status status) { | ||
34 | this.id = id; | 63 | this.id = id; |
35 | this.sender = sender; | 64 | this.sender = sender; |
36 | this.type = type; | 65 | this.type = type; |
37 | this.payload = payload; | 66 | this.payload = payload; |
67 | + this.status = status; | ||
38 | } | 68 | } |
39 | 69 | ||
40 | public long id() { | 70 | public long id() { |
... | @@ -53,12 +83,17 @@ public final class InternalMessage { | ... | @@ -53,12 +83,17 @@ public final class InternalMessage { |
53 | return payload; | 83 | return payload; |
54 | } | 84 | } |
55 | 85 | ||
86 | + public Status status() { | ||
87 | + return status; | ||
88 | + } | ||
89 | + | ||
56 | @Override | 90 | @Override |
57 | public String toString() { | 91 | public String toString() { |
58 | return MoreObjects.toStringHelper(this) | 92 | return MoreObjects.toStringHelper(this) |
59 | .add("id", id) | 93 | .add("id", id) |
60 | .add("type", type) | 94 | .add("type", type) |
61 | .add("sender", sender) | 95 | .add("sender", sender) |
96 | + .add("status", status) | ||
62 | .add("payload", ByteArraySizeHashPrinter.of(payload)) | 97 | .add("payload", ByteArraySizeHashPrinter.of(payload)) |
63 | .toString(); | 98 | .toString(); |
64 | } | 99 | } | ... | ... |
... | @@ -16,12 +16,15 @@ | ... | @@ -16,12 +16,15 @@ |
16 | package org.onosproject.store.cluster.messaging.impl; | 16 | package org.onosproject.store.cluster.messaging.impl; |
17 | 17 | ||
18 | import com.google.common.base.Charsets; | 18 | import com.google.common.base.Charsets; |
19 | + | ||
19 | import io.netty.buffer.ByteBuf; | 20 | import io.netty.buffer.ByteBuf; |
20 | import io.netty.channel.ChannelHandlerContext; | 21 | import io.netty.channel.ChannelHandlerContext; |
21 | import io.netty.handler.codec.ReplayingDecoder; | 22 | import io.netty.handler.codec.ReplayingDecoder; |
23 | + | ||
22 | import org.onlab.packet.IpAddress; | 24 | import org.onlab.packet.IpAddress; |
23 | import org.onlab.packet.IpAddress.Version; | 25 | import org.onlab.packet.IpAddress.Version; |
24 | import org.onosproject.store.cluster.messaging.Endpoint; | 26 | import org.onosproject.store.cluster.messaging.Endpoint; |
27 | +import org.onosproject.store.cluster.messaging.impl.InternalMessage.Status; | ||
25 | import org.slf4j.Logger; | 28 | import org.slf4j.Logger; |
26 | import org.slf4j.LoggerFactory; | 29 | import org.slf4j.LoggerFactory; |
27 | 30 | ||
... | @@ -44,6 +47,7 @@ public class MessageDecoder extends ReplayingDecoder<DecoderState> { | ... | @@ -44,6 +47,7 @@ public class MessageDecoder extends ReplayingDecoder<DecoderState> { |
44 | private int senderPort; | 47 | private int senderPort; |
45 | private int messageTypeLength; | 48 | private int messageTypeLength; |
46 | private String messageType; | 49 | private String messageType; |
50 | + private Status status; | ||
47 | private int contentLength; | 51 | private int contentLength; |
48 | 52 | ||
49 | public MessageDecoder(int correctPreamble) { | 53 | public MessageDecoder(int correctPreamble) { |
... | @@ -86,18 +90,27 @@ public class MessageDecoder extends ReplayingDecoder<DecoderState> { | ... | @@ -86,18 +90,27 @@ public class MessageDecoder extends ReplayingDecoder<DecoderState> { |
86 | byte[] messageTypeBytes = new byte[messageTypeLength]; | 90 | byte[] messageTypeBytes = new byte[messageTypeLength]; |
87 | buffer.readBytes(messageTypeBytes); | 91 | buffer.readBytes(messageTypeBytes); |
88 | messageType = new String(messageTypeBytes, Charsets.UTF_8); | 92 | messageType = new String(messageTypeBytes, Charsets.UTF_8); |
93 | + checkpoint(DecoderState.READ_MESSAGE_STATUS); | ||
94 | + case READ_MESSAGE_STATUS: | ||
95 | + status = Status.values()[buffer.readInt()]; | ||
89 | checkpoint(DecoderState.READ_CONTENT_LENGTH); | 96 | checkpoint(DecoderState.READ_CONTENT_LENGTH); |
90 | case READ_CONTENT_LENGTH: | 97 | case READ_CONTENT_LENGTH: |
91 | contentLength = buffer.readInt(); | 98 | contentLength = buffer.readInt(); |
92 | checkpoint(DecoderState.READ_CONTENT); | 99 | checkpoint(DecoderState.READ_CONTENT); |
93 | case READ_CONTENT: | 100 | case READ_CONTENT: |
94 | - //TODO Perform a sanity check on the size before allocating | 101 | + byte[] payload; |
95 | - byte[] payload = new byte[contentLength]; | 102 | + if (contentLength > 0) { |
96 | - buffer.readBytes(payload); | 103 | + //TODO Perform a sanity check on the size before allocating |
104 | + payload = new byte[contentLength]; | ||
105 | + buffer.readBytes(payload); | ||
106 | + } else { | ||
107 | + payload = new byte[0]; | ||
108 | + } | ||
97 | InternalMessage message = new InternalMessage(messageId, | 109 | InternalMessage message = new InternalMessage(messageId, |
98 | new Endpoint(senderIp, senderPort), | 110 | new Endpoint(senderIp, senderPort), |
99 | messageType, | 111 | messageType, |
100 | - payload); | 112 | + payload, |
113 | + status); | ||
101 | out.add(message); | 114 | out.add(message); |
102 | checkpoint(DecoderState.READ_MESSAGE_PREAMBLE); | 115 | checkpoint(DecoderState.READ_MESSAGE_PREAMBLE); |
103 | break; | 116 | break; | ... | ... |
... | @@ -75,6 +75,9 @@ public class MessageEncoder extends MessageToByteEncoder<InternalMessage> { | ... | @@ -75,6 +75,9 @@ public class MessageEncoder extends MessageToByteEncoder<InternalMessage> { |
75 | // write message type bytes | 75 | // write message type bytes |
76 | out.writeBytes(messageTypeBytes); | 76 | out.writeBytes(messageTypeBytes); |
77 | 77 | ||
78 | + // write message status value | ||
79 | + out.writeInt(message.status().ordinal()); | ||
80 | + | ||
78 | byte[] payload = message.payload(); | 81 | byte[] payload = message.payload(); |
79 | 82 | ||
80 | // write payload length | 83 | // write payload length | ... | ... |
... | @@ -16,12 +16,12 @@ | ... | @@ -16,12 +16,12 @@ |
16 | package org.onosproject.store.cluster.messaging.impl; | 16 | package org.onosproject.store.cluster.messaging.impl; |
17 | 17 | ||
18 | import com.google.common.base.Strings; | 18 | import com.google.common.base.Strings; |
19 | - | ||
20 | import com.google.common.cache.Cache; | 19 | import com.google.common.cache.Cache; |
21 | import com.google.common.cache.CacheBuilder; | 20 | import com.google.common.cache.CacheBuilder; |
22 | import com.google.common.cache.RemovalListener; | 21 | import com.google.common.cache.RemovalListener; |
23 | import com.google.common.cache.RemovalNotification; | 22 | import com.google.common.cache.RemovalNotification; |
24 | import com.google.common.util.concurrent.MoreExecutors; | 23 | import com.google.common.util.concurrent.MoreExecutors; |
24 | + | ||
25 | import io.netty.bootstrap.Bootstrap; | 25 | import io.netty.bootstrap.Bootstrap; |
26 | import io.netty.bootstrap.ServerBootstrap; | 26 | import io.netty.bootstrap.ServerBootstrap; |
27 | import io.netty.buffer.PooledByteBufAllocator; | 27 | import io.netty.buffer.PooledByteBufAllocator; |
... | @@ -41,6 +41,7 @@ import io.netty.channel.nio.NioEventLoopGroup; | ... | @@ -41,6 +41,7 @@ import io.netty.channel.nio.NioEventLoopGroup; |
41 | import io.netty.channel.socket.SocketChannel; | 41 | import io.netty.channel.socket.SocketChannel; |
42 | import io.netty.channel.socket.nio.NioServerSocketChannel; | 42 | import io.netty.channel.socket.nio.NioServerSocketChannel; |
43 | import io.netty.channel.socket.nio.NioSocketChannel; | 43 | import io.netty.channel.socket.nio.NioSocketChannel; |
44 | + | ||
44 | import org.apache.commons.pool.KeyedPoolableObjectFactory; | 45 | import org.apache.commons.pool.KeyedPoolableObjectFactory; |
45 | import org.apache.commons.pool.impl.GenericKeyedObjectPool; | 46 | import org.apache.commons.pool.impl.GenericKeyedObjectPool; |
46 | import org.apache.felix.scr.annotations.Activate; | 47 | import org.apache.felix.scr.annotations.Activate; |
... | @@ -53,7 +54,9 @@ import org.onlab.util.Tools; | ... | @@ -53,7 +54,9 @@ import org.onlab.util.Tools; |
53 | import org.onosproject.cluster.ClusterMetadataService; | 54 | import org.onosproject.cluster.ClusterMetadataService; |
54 | import org.onosproject.cluster.ControllerNode; | 55 | import org.onosproject.cluster.ControllerNode; |
55 | import org.onosproject.store.cluster.messaging.Endpoint; | 56 | import org.onosproject.store.cluster.messaging.Endpoint; |
57 | +import org.onosproject.store.cluster.messaging.MessagingException; | ||
56 | import org.onosproject.store.cluster.messaging.MessagingService; | 58 | import org.onosproject.store.cluster.messaging.MessagingService; |
59 | +import org.onosproject.store.cluster.messaging.impl.InternalMessage.Status; | ||
57 | import org.slf4j.Logger; | 60 | import org.slf4j.Logger; |
58 | import org.slf4j.LoggerFactory; | 61 | import org.slf4j.LoggerFactory; |
59 | 62 | ||
... | @@ -61,10 +64,12 @@ import javax.net.ssl.KeyManagerFactory; | ... | @@ -61,10 +64,12 @@ import javax.net.ssl.KeyManagerFactory; |
61 | import javax.net.ssl.SSLContext; | 64 | import javax.net.ssl.SSLContext; |
62 | import javax.net.ssl.SSLEngine; | 65 | import javax.net.ssl.SSLEngine; |
63 | import javax.net.ssl.TrustManagerFactory; | 66 | import javax.net.ssl.TrustManagerFactory; |
67 | + | ||
64 | import java.io.FileInputStream; | 68 | import java.io.FileInputStream; |
65 | import java.io.IOException; | 69 | import java.io.IOException; |
66 | import java.security.KeyStore; | 70 | import java.security.KeyStore; |
67 | import java.util.Map; | 71 | import java.util.Map; |
72 | +import java.util.Optional; | ||
68 | import java.util.concurrent.CompletableFuture; | 73 | import java.util.concurrent.CompletableFuture; |
69 | import java.util.concurrent.ConcurrentHashMap; | 74 | import java.util.concurrent.ConcurrentHashMap; |
70 | import java.util.concurrent.Executor; | 75 | import java.util.concurrent.Executor; |
... | @@ -267,18 +272,14 @@ public class NettyMessagingManager implements MessagingService { | ... | @@ -267,18 +272,14 @@ public class NettyMessagingManager implements MessagingService { |
267 | @Override | 272 | @Override |
268 | public void registerHandler(String type, BiFunction<Endpoint, byte[], byte[]> handler, Executor executor) { | 273 | public void registerHandler(String type, BiFunction<Endpoint, byte[], byte[]> handler, Executor executor) { |
269 | handlers.put(type, message -> executor.execute(() -> { | 274 | handlers.put(type, message -> executor.execute(() -> { |
270 | - byte[] responsePayload = handler.apply(message.sender(), message.payload()); | 275 | + byte[] responsePayload = null; |
271 | - if (responsePayload != null) { | 276 | + Status status = Status.OK; |
272 | - InternalMessage response = new InternalMessage(message.id(), | 277 | + try { |
273 | - localEp, | 278 | + responsePayload = handler.apply(message.sender(), message.payload()); |
274 | - REPLY_MESSAGE_TYPE, | 279 | + } catch (Exception e) { |
275 | - responsePayload); | 280 | + status = Status.ERROR_HANDLER_EXCEPTION; |
276 | - sendAsync(message.sender(), response).whenComplete((result, error) -> { | ||
277 | - if (error != null) { | ||
278 | - log.debug("Failed to respond", error); | ||
279 | - } | ||
280 | - }); | ||
281 | } | 281 | } |
282 | + sendReply(message, status, Optional.ofNullable(responsePayload)); | ||
282 | })); | 283 | })); |
283 | } | 284 | } |
284 | 285 | ||
... | @@ -286,17 +287,8 @@ public class NettyMessagingManager implements MessagingService { | ... | @@ -286,17 +287,8 @@ public class NettyMessagingManager implements MessagingService { |
286 | public void registerHandler(String type, BiFunction<Endpoint, byte[], CompletableFuture<byte[]>> handler) { | 287 | public void registerHandler(String type, BiFunction<Endpoint, byte[], CompletableFuture<byte[]>> handler) { |
287 | handlers.put(type, message -> { | 288 | handlers.put(type, message -> { |
288 | handler.apply(message.sender(), message.payload()).whenComplete((result, error) -> { | 289 | handler.apply(message.sender(), message.payload()).whenComplete((result, error) -> { |
289 | - if (error == null) { | 290 | + Status status = error == null ? Status.OK : Status.ERROR_HANDLER_EXCEPTION; |
290 | - InternalMessage response = new InternalMessage(message.id(), | 291 | + sendReply(message, status, Optional.ofNullable(result)); |
291 | - localEp, | ||
292 | - REPLY_MESSAGE_TYPE, | ||
293 | - result); | ||
294 | - sendAsync(message.sender(), response).whenComplete((r, e) -> { | ||
295 | - if (e != null) { | ||
296 | - log.debug("Failed to respond", e); | ||
297 | - } | ||
298 | - }); | ||
299 | - } | ||
300 | }); | 292 | }); |
301 | }); | 293 | }); |
302 | } | 294 | } |
... | @@ -500,9 +492,15 @@ public class NettyMessagingManager implements MessagingService { | ... | @@ -500,9 +492,15 @@ public class NettyMessagingManager implements MessagingService { |
500 | Callback callback = | 492 | Callback callback = |
501 | callbacks.getIfPresent(message.id()); | 493 | callbacks.getIfPresent(message.id()); |
502 | if (callback != null) { | 494 | if (callback != null) { |
503 | - callback.complete(message.payload()); | 495 | + if (message.status() == Status.OK) { |
496 | + callback.complete(message.payload()); | ||
497 | + } else if (message.status() == Status.ERROR_NO_HANDLER) { | ||
498 | + callback.completeExceptionally(new MessagingException.NoRemoteHandler()); | ||
499 | + } else if (message.status() == Status.ERROR_HANDLER_EXCEPTION) { | ||
500 | + callback.completeExceptionally(new MessagingException.RemoteHandlerFailure()); | ||
501 | + } | ||
504 | } else { | 502 | } else { |
505 | - log.warn("Received a reply for message id:[{}]. " | 503 | + log.debug("Received a reply for message id:[{}]. " |
506 | + " from {}. But was unable to locate the" | 504 | + " from {}. But was unable to locate the" |
507 | + " request handle", message.id(), message.sender()); | 505 | + " request handle", message.id(), message.sender()); |
508 | } | 506 | } |
... | @@ -515,10 +513,24 @@ public class NettyMessagingManager implements MessagingService { | ... | @@ -515,10 +513,24 @@ public class NettyMessagingManager implements MessagingService { |
515 | if (handler != null) { | 513 | if (handler != null) { |
516 | handler.accept(message); | 514 | handler.accept(message); |
517 | } else { | 515 | } else { |
518 | - log.debug("No handler registered for {}", type); | 516 | + log.debug("No handler for message type {}", message.type(), message.sender()); |
517 | + sendReply(message, Status.ERROR_NO_HANDLER, Optional.empty()); | ||
519 | } | 518 | } |
520 | } | 519 | } |
521 | 520 | ||
521 | + private void sendReply(InternalMessage message, Status status, Optional<byte[]> responsePayload) { | ||
522 | + InternalMessage response = new InternalMessage(message.id(), | ||
523 | + localEp, | ||
524 | + REPLY_MESSAGE_TYPE, | ||
525 | + responsePayload.orElse(new byte[0]), | ||
526 | + status); | ||
527 | + sendAsync(message.sender(), response).whenComplete((result, error) -> { | ||
528 | + if (error != null) { | ||
529 | + log.debug("Failed to respond", error); | ||
530 | + } | ||
531 | + }); | ||
532 | + } | ||
533 | + | ||
522 | private final class Callback { | 534 | private final class Callback { |
523 | private final CompletableFuture<byte[]> future; | 535 | private final CompletableFuture<byte[]> future; |
524 | private final Executor executor; | 536 | private final Executor executor; | ... | ... |
-
Please register or login to post a comment