홍길동 / onos

Go to a project
Toggle navigation Toggle navigation pinning
Yuta HIGUCHI
Committed by Yuta Higuchi
03e77e97 1 parent 91768e30

ClusterMessagingProtocol: stop processing in netty handler thread 

- Fix for io.netty.util.concurrent.BlockingOperationException

Change-Id: Ie0f4dee2c3a49aa4b03674f6f7678f32fcf07a44
Inline Side-by-side
Showing 2 changed files with 128 additions and 53 deletions
...@@ -4,12 +4,12 @@ import static com.google.common.base.Verify.verifyNotNull; ...@@ -4,12 +4,12 @@ import static com.google.common.base.Verify.verifyNotNull;
4 import static org.onlab.onos.store.service.impl.ClusterMessagingProtocol.DB_SERIALIZER; 4 import static org.onlab.onos.store.service.impl.ClusterMessagingProtocol.DB_SERIALIZER;
5 import static org.onlab.util.Tools.namedThreads; 5 import static org.onlab.util.Tools.namedThreads;
6 import static org.slf4j.LoggerFactory.getLogger; 6 import static org.slf4j.LoggerFactory.getLogger;
7 +import static java.util.concurrent.Executors.newCachedThreadPool;
7 8
8 import java.io.IOException; 9 import java.io.IOException;
9 import java.util.concurrent.CompletableFuture; 10 import java.util.concurrent.CompletableFuture;
10 import java.util.concurrent.ExecutionException; 11 import java.util.concurrent.ExecutionException;
11 import java.util.concurrent.ExecutorService; 12 import java.util.concurrent.ExecutorService;
12 -import java.util.concurrent.Executors;
13 import java.util.concurrent.TimeUnit; 13 import java.util.concurrent.TimeUnit;
14 import java.util.concurrent.TimeoutException; 14 import java.util.concurrent.TimeoutException;
15 import java.util.concurrent.atomic.AtomicBoolean; 15 import java.util.concurrent.atomic.AtomicBoolean;
...@@ -49,9 +49,7 @@ public class ClusterMessagingProtocolClient implements ProtocolClient { ...@@ -49,9 +49,7 @@ public class ClusterMessagingProtocolClient implements ProtocolClient {
49 private ControllerNode remoteNode; 49 private ControllerNode remoteNode;
50 private final AtomicBoolean connectionOK = new AtomicBoolean(true); 50 private final AtomicBoolean connectionOK = new AtomicBoolean(true);
51 51
52 - // TODO: make this non-static and stop on close 52 + private ExecutorService pool;
53 - private static final ExecutorService THREAD_POOL
54 - = Executors.newCachedThreadPool(namedThreads("copycat-netty-messaging-%d"));
55 53
56 public ClusterMessagingProtocolClient( 54 public ClusterMessagingProtocolClient(
57 ClusterService clusterService, 55 ClusterService clusterService,
...@@ -87,11 +85,19 @@ public class ClusterMessagingProtocolClient implements ProtocolClient { ...@@ -87,11 +85,19 @@ public class ClusterMessagingProtocolClient implements ProtocolClient {
87 85
88 @Override 86 @Override
89 public synchronized CompletableFuture<Void> connect() { 87 public synchronized CompletableFuture<Void> connect() {
88 + if (pool == null || pool.isShutdown()) {
89 + // TODO include remote name?
90 + pool = newCachedThreadPool(namedThreads("copycat-netty-messaging-client-%d"));
91 + }
90 return CompletableFuture.completedFuture(null); 92 return CompletableFuture.completedFuture(null);
91 } 93 }
92 94
93 @Override 95 @Override
94 public synchronized CompletableFuture<Void> close() { 96 public synchronized CompletableFuture<Void> close() {
97 + if (pool != null) {
98 + pool.shutdownNow();
99 + pool = null;
100 + }
95 return CompletableFuture.completedFuture(null); 101 return CompletableFuture.completedFuture(null);
96 } 102 }
97 103
...@@ -112,7 +118,11 @@ public class ClusterMessagingProtocolClient implements ProtocolClient { ...@@ -112,7 +118,11 @@ public class ClusterMessagingProtocolClient implements ProtocolClient {
112 118
113 private <I, O> CompletableFuture<O> requestReply(I request) { 119 private <I, O> CompletableFuture<O> requestReply(I request) {
114 CompletableFuture<O> future = new CompletableFuture<>(); 120 CompletableFuture<O> future = new CompletableFuture<>();
115 - THREAD_POOL.submit(new RPCTask<I, O>(request, future)); 121 + if (pool == null) {
122 + log.info("Attempted to use closed client, connecting now. {}", request);
123 + connect();
124 + }
125 + pool.submit(new RPCTask<I, O>(request, future));
116 return future; 126 return future;
117 } 127 }
118 128
......
1 package org.onlab.onos.store.service.impl; 1 package org.onlab.onos.store.service.impl;
2 2
3 +import static java.util.concurrent.Executors.newCachedThreadPool;
4 +import static org.onlab.util.Tools.namedThreads;
3 import static org.slf4j.LoggerFactory.getLogger; 5 import static org.slf4j.LoggerFactory.getLogger;
6 +import static org.onlab.onos.store.service.impl.ClusterMessagingProtocol.*;
7 +import static org.onlab.onos.store.service.impl.ClusterMessagingProtocol.DB_SERIALIZER;
4 8
5 import java.util.concurrent.CompletableFuture; 9 import java.util.concurrent.CompletableFuture;
10 +import java.util.concurrent.ExecutorService;
6 import java.util.function.BiConsumer; 11 import java.util.function.BiConsumer;
7 12
8 import net.kuujo.copycat.protocol.PingRequest; 13 import net.kuujo.copycat.protocol.PingRequest;
9 -import net.kuujo.copycat.protocol.PingResponse;
10 import net.kuujo.copycat.protocol.PollRequest; 14 import net.kuujo.copycat.protocol.PollRequest;
11 -import net.kuujo.copycat.protocol.PollResponse;
12 import net.kuujo.copycat.protocol.RequestHandler; 15 import net.kuujo.copycat.protocol.RequestHandler;
13 import net.kuujo.copycat.protocol.SubmitRequest; 16 import net.kuujo.copycat.protocol.SubmitRequest;
14 -import net.kuujo.copycat.protocol.SubmitResponse;
15 import net.kuujo.copycat.protocol.SyncRequest; 17 import net.kuujo.copycat.protocol.SyncRequest;
16 -import net.kuujo.copycat.protocol.SyncResponse;
17 import net.kuujo.copycat.spi.protocol.ProtocolServer; 18 import net.kuujo.copycat.spi.protocol.ProtocolServer;
18 19
19 import org.onlab.onos.store.cluster.messaging.ClusterCommunicationService; 20 import org.onlab.onos.store.cluster.messaging.ClusterCommunicationService;
...@@ -27,12 +28,15 @@ import org.slf4j.Logger; ...@@ -27,12 +28,15 @@ import org.slf4j.Logger;
27 public class ClusterMessagingProtocolServer implements ProtocolServer { 28 public class ClusterMessagingProtocolServer implements ProtocolServer {
28 29
29 private final Logger log = getLogger(getClass()); 30 private final Logger log = getLogger(getClass());
31 +
32 + private final ClusterCommunicationService clusterCommunicator;
33 +
30 private volatile RequestHandler handler; 34 private volatile RequestHandler handler;
31 - private ClusterCommunicationService clusterCommunicator; 35 +
36 + private ExecutorService pool;
32 37
33 public ClusterMessagingProtocolServer(ClusterCommunicationService clusterCommunicator) { 38 public ClusterMessagingProtocolServer(ClusterCommunicationService clusterCommunicator) {
34 this.clusterCommunicator = clusterCommunicator; 39 this.clusterCommunicator = clusterCommunicator;
35 -
36 } 40 }
37 41
38 @Override 42 @Override
...@@ -42,67 +46,128 @@ public class ClusterMessagingProtocolServer implements ProtocolServer { ...@@ -42,67 +46,128 @@ public class ClusterMessagingProtocolServer implements ProtocolServer {
42 46
43 @Override 47 @Override
44 public CompletableFuture<Void> listen() { 48 public CompletableFuture<Void> listen() {
45 - clusterCommunicator.addSubscriber(ClusterMessagingProtocol.COPYCAT_PING, 49 + if (pool == null || pool.isShutdown()) {
46 - new CopycatMessageHandler<PingRequest>()); 50 + pool = newCachedThreadPool(namedThreads("copycat-netty-messaging-server-%d"));
47 - clusterCommunicator.addSubscriber(ClusterMessagingProtocol.COPYCAT_SYNC, 51 + }
48 - new CopycatMessageHandler<SyncRequest>()); 52 +
49 - clusterCommunicator.addSubscriber(ClusterMessagingProtocol.COPYCAT_POLL, 53 + clusterCommunicator.addSubscriber(COPYCAT_PING, new PingHandler());
50 - new CopycatMessageHandler<PollRequest>()); 54 + clusterCommunicator.addSubscriber(COPYCAT_SYNC, new SyncHandler());
51 - clusterCommunicator.addSubscriber(ClusterMessagingProtocol.COPYCAT_SUBMIT, 55 + clusterCommunicator.addSubscriber(COPYCAT_POLL, new PollHandler());
52 - new CopycatMessageHandler<SubmitRequest>()); 56 + clusterCommunicator.addSubscriber(COPYCAT_SUBMIT, new SubmitHandler());
53 return CompletableFuture.completedFuture(null); 57 return CompletableFuture.completedFuture(null);
54 } 58 }
55 59
56 @Override 60 @Override
57 public CompletableFuture<Void> close() { 61 public CompletableFuture<Void> close() {
58 - clusterCommunicator.removeSubscriber(ClusterMessagingProtocol.COPYCAT_PING); 62 + clusterCommunicator.removeSubscriber(COPYCAT_PING);
59 - clusterCommunicator.removeSubscriber(ClusterMessagingProtocol.COPYCAT_SYNC); 63 + clusterCommunicator.removeSubscriber(COPYCAT_SYNC);
60 - clusterCommunicator.removeSubscriber(ClusterMessagingProtocol.COPYCAT_POLL); 64 + clusterCommunicator.removeSubscriber(COPYCAT_POLL);
61 - clusterCommunicator.removeSubscriber(ClusterMessagingProtocol.COPYCAT_SUBMIT); 65 + clusterCommunicator.removeSubscriber(COPYCAT_SUBMIT);
66 + if (pool != null) {
67 + pool.shutdownNow();
68 + pool = null;
69 + }
62 return CompletableFuture.completedFuture(null); 70 return CompletableFuture.completedFuture(null);
63 } 71 }
64 72
65 - private class CopycatMessageHandler<T> implements ClusterMessageHandler { 73 + private final class PingHandler extends CopycatMessageHandler<PingRequest> {
74 +
75 + @Override
76 + public void raftHandle(PingRequest request, ClusterMessage message) {
77 + pool.submit(new Runnable() {
78 +
79 + @Override
80 + public void run() {
81 + currentHandler().ping(request)
82 + .whenComplete(new PostExecutionTask<>(message));
83 + }
84 + });
85 + }
86 + }
87 +
88 + private final class SyncHandler extends CopycatMessageHandler<SyncRequest> {
89 +
90 + @Override
91 + public void raftHandle(SyncRequest request, ClusterMessage message) {
92 + pool.submit(new Runnable() {
93 +
94 + @Override
95 + public void run() {
96 + currentHandler().sync(request)
97 + .whenComplete(new PostExecutionTask<>(message));
98 + }
99 + });
100 + }
101 + }
102 +
103 + private final class PollHandler extends CopycatMessageHandler<PollRequest> {
104 +
105 + @Override
106 + public void raftHandle(PollRequest request, ClusterMessage message) {
107 + pool.submit(new Runnable() {
108 +
109 + @Override
110 + public void run() {
111 + currentHandler().poll(request)
112 + .whenComplete(new PostExecutionTask<>(message));
113 + }
114 + });
115 + }
116 + }
117 +
118 + private final class SubmitHandler extends CopycatMessageHandler<SubmitRequest> {
119 +
120 + @Override
121 + public void raftHandle(SubmitRequest request, ClusterMessage message) {
122 + pool.submit(new Runnable() {
123 +
124 + @Override
125 + public void run() {
126 + currentHandler().submit(request)
127 + .whenComplete(new PostExecutionTask<>(message));
128 + }
129 + });
130 + }
131 + }
132 +
133 + private abstract class CopycatMessageHandler<T> implements ClusterMessageHandler {
134 +
135 + public abstract void raftHandle(T request, ClusterMessage message);
66 136
67 @Override 137 @Override
68 public void handle(ClusterMessage message) { 138 public void handle(ClusterMessage message) {
69 - T request = ClusterMessagingProtocol.DB_SERIALIZER.decode(message.payload()); 139 + T request = DB_SERIALIZER.decode(message.payload());
70 - if (handler == null) { 140 + raftHandle(request, message);
141 + }
142 +
143 + RequestHandler currentHandler() {
144 + RequestHandler currentHandler = handler;
145 + if (currentHandler == null) {
71 // there is a slight window of time during state transition, 146 // there is a slight window of time during state transition,
72 // where handler becomes null 147 // where handler becomes null
148 + long sleepMs = 1;
73 for (int i = 0; i < 10; ++i) { 149 for (int i = 0; i < 10; ++i) {
74 - if (handler != null) { 150 + currentHandler = handler;
151 + if (currentHandler != null) {
75 break; 152 break;
76 } 153 }
77 try { 154 try {
78 - Thread.sleep(1); 155 + sleepMs <<= 1;
156 + Thread.sleep(sleepMs);
79 } catch (InterruptedException e) { 157 } catch (InterruptedException e) {
80 - log.trace("Exception", e); 158 + log.error("Interrupted", e);
159 + return handler;
81 } 160 }
82 } 161 }
83 - if (handler == null) { 162 + if (currentHandler == null) {
84 log.error("There was no handler registered!"); 163 log.error("There was no handler registered!");
85 - return; 164 + return handler;
86 - } 165 + }
87 - }
88 - if (request.getClass().equals(PingRequest.class)) {
89 - handler.ping((PingRequest) request)
90 - .whenComplete(new PostExecutionTask<PingResponse>(message));
91 - } else if (request.getClass().equals(PollRequest.class)) {
92 - handler.poll((PollRequest) request)
93 - .whenComplete(new PostExecutionTask<PollResponse>(message));
94 - } else if (request.getClass().equals(SyncRequest.class)) {
95 - handler.sync((SyncRequest) request)
96 - .whenComplete(new PostExecutionTask<SyncResponse>(message));
97 - } else if (request.getClass().equals(SubmitRequest.class)) {
98 - handler.submit((SubmitRequest) request)
99 - .whenComplete(new PostExecutionTask<SubmitResponse>(message));
100 - } else {
101 - throw new IllegalStateException("Unknown request type: " + request.getClass().getName());
102 } 166 }
167 + return currentHandler;
103 } 168 }
104 169
105 - private class PostExecutionTask<R> implements BiConsumer<R, Throwable> { 170 + final class PostExecutionTask<R> implements BiConsumer<R, Throwable> {
106 171
107 private final ClusterMessage message; 172 private final ClusterMessage message;
108 173
...@@ -111,15 +176,15 @@ public class ClusterMessagingProtocolServer implements ProtocolServer { ...@@ -111,15 +176,15 @@ public class ClusterMessagingProtocolServer implements ProtocolServer {
111 } 176 }
112 177
113 @Override 178 @Override
114 - public void accept(R response, Throwable t) { 179 + public void accept(R response, Throwable error) {
115 - if (t != null) { 180 + if (error != null) {
116 - log.error("Processing for " + message.subject() + " failed.", t); 181 + log.error("Processing {} failed.", message.subject(), error);
117 } else { 182 } else {
118 try { 183 try {
119 log.trace("responding to {}", message.subject()); 184 log.trace("responding to {}", message.subject());
120 - message.respond(ClusterMessagingProtocol.DB_SERIALIZER.encode(response)); 185 + message.respond(DB_SERIALIZER.encode(response));
121 } catch (Exception e) { 186 } catch (Exception e) {
122 - log.error("Failed to respond to " + response.getClass().getName(), e); 187 + log.error("Failed responding with {}", response.getClass().getName(), e);
123 } 188 }
124 } 189 }
125 } 190 }
......