Skip to content

Commit a61ef74

Browse files
yunwzyunwz
committed
add test unit for DefaultRequestFuture#cancel().
1 parent a9e2d8e commit a61ef74

4 files changed

Lines changed: 128 additions & 22 deletions

File tree

‎api/src/main/java/com/alibaba/nacos/api/remote/DefaultRequestFuture.java‎

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public class DefaultRequestFuture implements RequestFuture {
4848

4949
private ScheduledFuture timeoutFuture;
5050

51-
TimeoutInnerTrigger timeoutInnerTrigger;
51+
FutureTrigger futureTrigger;
5252

5353
/**
5454
* Getter method for property <tt>requestCallBack</tt>.
@@ -73,16 +73,16 @@ public DefaultRequestFuture(String connectionId, String requestId) {
7373
}
7474

7575
public DefaultRequestFuture(String connectionId, String requestId, RequestCallBack requestCallBack,
76-
TimeoutInnerTrigger timeoutInnerTrigger) {
76+
FutureTrigger futureTrigger) {
7777
this.timeStamp = System.currentTimeMillis();
7878
this.requestCallBack = requestCallBack;
7979
this.requestId = requestId;
8080
this.connectionId = connectionId;
8181
if (requestCallBack != null) {
82-
this.timeoutFuture = RpcScheduledExecutor.TIMEOUT_SCHEDULER
83-
.schedule(new TimeoutHandler(), requestCallBack.getTimeout(), TimeUnit.MILLISECONDS);
82+
this.timeoutFuture = RpcScheduledExecutor.TIMEOUT_SCHEDULER.schedule(new TimeoutHandler(),
83+
requestCallBack.getTimeout(), TimeUnit.MILLISECONDS);
8484
}
85-
this.timeoutInnerTrigger = timeoutInnerTrigger;
85+
this.futureTrigger = futureTrigger;
8686
}
8787

8888
public void setResponse(final Response response) {
@@ -161,8 +161,8 @@ public Response get(long timeout) throws TimeoutException, InterruptedException
161161
if (isDone) {
162162
return response;
163163
} else {
164-
if (timeoutInnerTrigger != null) {
165-
timeoutInnerTrigger.triggerOnTimeout();
164+
if (timeoutFuture == null && futureTrigger != null) {
165+
futureTrigger.triggerOnTimeout();
166166
}
167167
throw new TimeoutException(
168168
"request timeout after " + timeout + " milliseconds, requestId=" + requestId + ", connectionId="
@@ -192,18 +192,35 @@ public void run() {
192192
setFailResult(new TimeoutException(
193193
"Timeout After " + requestCallBack.getTimeout() + " milliseconds, requestId=" + requestId
194194
+ ", connectionId=" + connectionId));
195-
if (timeoutInnerTrigger != null) {
196-
timeoutInnerTrigger.triggerOnTimeout();
195+
if (futureTrigger != null) {
196+
futureTrigger.triggerOnTimeout();
197197
}
198198
}
199199
}
200200

201-
public interface TimeoutInnerTrigger {
201+
/**
202+
* Cleaning something while request has been failed, canceled, timeout.
203+
*/
204+
public interface FutureTrigger {
205+
206+
/**
207+
* default trigger for {@link #triggerOnTimeout()} and {@link #triggerOnCancel()}.
208+
*/
209+
void defaultTrigger();
202210

203211
/**
204212
* triggered on timeout .
205213
*/
206-
void triggerOnTimeout();
214+
default void triggerOnTimeout() {
215+
defaultTrigger();
216+
}
217+
218+
/**
219+
* triggered on cancel.
220+
*/
221+
default void triggerOnCancel() {
222+
defaultTrigger();
223+
}
207224

208225
}
209226

@@ -215,4 +232,25 @@ public interface TimeoutInnerTrigger {
215232
public String getConnectionId() {
216233
return connectionId;
217234
}
235+
236+
/**
237+
* Cancel the request. It should be called in
238+
* {@link com.alibaba.nacos.core.remote.grpc.GrpcConnection#sendRequestInner}
239+
* NOTE: For sync requests(which without requestCallBack), the cancel operation is always invalid.
240+
*
241+
* @param mayInterruptIfRunning whether to interrupt the thread
242+
*/
243+
public void cancel(boolean mayInterruptIfRunning) {
244+
synchronized (this) {
245+
notifyAll();
246+
}
247+
// cancel timeout task.
248+
if (timeoutFuture != null && !timeoutFuture.isDone()) {
249+
boolean cancel = timeoutFuture.cancel(mayInterruptIfRunning);
250+
if (cancel && futureTrigger != null) {
251+
futureTrigger.triggerOnCancel();
252+
}
253+
}
254+
255+
}
218256
}

‎api/src/test/java/com/alibaba/nacos/api/remote/DefaultRequestFutureTest.java‎

Lines changed: 73 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ void testSyncGetResponseFailureWithTimeout() throws InterruptedException, Timeou
143143

144144
@Test
145145
void testSyncGetResponseSuccessByTriggerWithoutTimeout() throws InterruptedException {
146-
MockTimeoutInnerTrigger trigger = new MockTimeoutInnerTrigger();
146+
MockFutureTrigger trigger = new MockFutureTrigger();
147147
DefaultRequestFuture requestFuture = new DefaultRequestFuture(CONNECTION_ID, REQUEST_ID, null, trigger);
148148
new Thread(() -> {
149149
try {
@@ -163,7 +163,7 @@ void testSyncGetResponseSuccessByTriggerWithoutTimeout() throws InterruptedExcep
163163

164164
@Test
165165
void testSyncGetResponseFailureByTriggerWithoutTimeout() throws InterruptedException {
166-
MockTimeoutInnerTrigger trigger = new MockTimeoutInnerTrigger();
166+
MockFutureTrigger trigger = new MockFutureTrigger();
167167
DefaultRequestFuture requestFuture = new DefaultRequestFuture(CONNECTION_ID, REQUEST_ID, null, trigger);
168168
new Thread(() -> {
169169
try {
@@ -183,7 +183,7 @@ void testSyncGetResponseFailureByTriggerWithoutTimeout() throws InterruptedExcep
183183

184184
@Test
185185
void testSyncGetResponseSuccessByTriggerWithTimeout() throws InterruptedException, TimeoutException {
186-
MockTimeoutInnerTrigger trigger = new MockTimeoutInnerTrigger();
186+
MockFutureTrigger trigger = new MockFutureTrigger();
187187
DefaultRequestFuture requestFuture = new DefaultRequestFuture(CONNECTION_ID, REQUEST_ID, null, trigger);
188188
new Thread(() -> {
189189
try {
@@ -199,12 +199,13 @@ void testSyncGetResponseSuccessByTriggerWithTimeout() throws InterruptedExceptio
199199
assertTrue(requestFuture.isDone());
200200
assertTrue(requestFuture.getTimeStamp() >= timestamp);
201201
assertFalse(trigger.isTimeout);
202+
assertFalse(trigger.isCancel);
202203
}
203204

204205
@Test
205206
void testSyncGetResponseFailureByTriggerWithTimeout() throws InterruptedException, TimeoutException {
206207
assertThrows(TimeoutException.class, () -> {
207-
MockTimeoutInnerTrigger trigger = new MockTimeoutInnerTrigger();
208+
MockFutureTrigger trigger = new MockFutureTrigger();
208209
DefaultRequestFuture requestFuture = new DefaultRequestFuture(CONNECTION_ID, REQUEST_ID, null, trigger);
209210
try {
210211
requestFuture.get(100L);
@@ -216,7 +217,7 @@ void testSyncGetResponseFailureByTriggerWithTimeout() throws InterruptedExceptio
216217

217218
@Test
218219
void testASyncGetResponseSuccessWithoutTimeout() throws InterruptedException {
219-
MockTimeoutInnerTrigger trigger = new MockTimeoutInnerTrigger();
220+
MockFutureTrigger trigger = new MockFutureTrigger();
220221
MockRequestCallback callback = new MockRequestCallback(200L);
221222
DefaultRequestFuture requestFuture = new DefaultRequestFuture(CONNECTION_ID, REQUEST_ID, callback, trigger);
222223
new Thread(() -> {
@@ -230,12 +231,13 @@ void testASyncGetResponseSuccessWithoutTimeout() throws InterruptedException {
230231
assertEquals(response, callback.response);
231232
assertNull(callback.exception);
232233
assertFalse(trigger.isTimeout);
234+
assertFalse(trigger.isTimeout);
233235
assertEquals(callback, requestFuture.getRequestCallBack());
234236
}
235237

236238
@Test
237239
void testASyncGetResponseSuccessWithoutTimeoutByExecutor() throws InterruptedException {
238-
MockTimeoutInnerTrigger trigger = new MockTimeoutInnerTrigger();
240+
MockFutureTrigger trigger = new MockFutureTrigger();
239241
MockRequestCallback callback = new MockRequestCallback(executor, 200L);
240242
DefaultRequestFuture requestFuture = new DefaultRequestFuture(CONNECTION_ID, REQUEST_ID, callback, trigger);
241243
new Thread(() -> {
@@ -252,7 +254,7 @@ void testASyncGetResponseSuccessWithoutTimeoutByExecutor() throws InterruptedExc
252254

253255
@Test
254256
void testASyncGetResponseFailureWithoutTimeout() throws InterruptedException {
255-
MockTimeoutInnerTrigger trigger = new MockTimeoutInnerTrigger();
257+
MockFutureTrigger trigger = new MockFutureTrigger();
256258
MockRequestCallback callback = new MockRequestCallback(1000L);
257259
DefaultRequestFuture requestFuture = new DefaultRequestFuture(CONNECTION_ID, REQUEST_ID, callback, trigger);
258260
new Thread(() -> {
@@ -271,7 +273,7 @@ void testASyncGetResponseFailureWithoutTimeout() throws InterruptedException {
271273

272274
@Test
273275
void testASyncGetResponseFailureWithTimeout() throws InterruptedException {
274-
MockTimeoutInnerTrigger trigger = new MockTimeoutInnerTrigger();
276+
MockFutureTrigger trigger = new MockFutureTrigger();
275277
MockRequestCallback callback = new MockRequestCallback(100L);
276278
final DefaultRequestFuture requestFuture = new DefaultRequestFuture(CONNECTION_ID, REQUEST_ID, callback,
277279
trigger);
@@ -282,14 +284,75 @@ void testASyncGetResponseFailureWithTimeout() throws InterruptedException {
282284
assertEquals(callback, requestFuture.getRequestCallBack());
283285
}
284286

285-
private class MockTimeoutInnerTrigger implements DefaultRequestFuture.TimeoutInnerTrigger {
287+
@Test
288+
void testSyncRequestFutureCancelFailedWithTimeout() throws InterruptedException {
289+
MockFutureTrigger trigger = new MockFutureTrigger();
290+
final DefaultRequestFuture requestFuture = new DefaultRequestFuture(CONNECTION_ID, REQUEST_ID, null, trigger);
291+
assertThrows(TimeoutException.class, () -> requestFuture.get(100L));
292+
requestFuture.cancel(true);
293+
assertTrue(trigger.isTimeout);
294+
assertFalse(trigger.isCancel);
295+
}
296+
297+
@Test
298+
void testSyncRequestFutureCancelFailed() throws InterruptedException {
299+
MockFutureTrigger trigger = new MockFutureTrigger();
300+
final DefaultRequestFuture requestFuture = new DefaultRequestFuture(CONNECTION_ID, REQUEST_ID, null, trigger);
301+
requestFuture.cancel(true);
302+
assertFalse(trigger.isTimeout);
303+
assertFalse(trigger.isCancel);
304+
}
305+
306+
@Test
307+
void testASyncRequestFutureCancelFailedWithTrigger() throws InterruptedException {
308+
MockFutureTrigger trigger = new MockFutureTrigger();
309+
MockRequestCallback callback = new MockRequestCallback(100L);
310+
final DefaultRequestFuture requestFuture = new DefaultRequestFuture(CONNECTION_ID, REQUEST_ID, callback,
311+
trigger);
312+
TimeUnit.MILLISECONDS.sleep(500L);
313+
requestFuture.cancel(true);
314+
assertNull(callback.response);
315+
assertTrue(callback.exception instanceof TimeoutException);
316+
assertTrue(trigger.isTimeout);
317+
assertFalse(trigger.isCancel);
318+
assertEquals(callback, requestFuture.getRequestCallBack());
319+
}
320+
321+
@Test
322+
void testASyncRequestFutureCancelSuccessWithTrigger() throws InterruptedException {
323+
MockFutureTrigger trigger = new MockFutureTrigger();
324+
MockRequestCallback callback = new MockRequestCallback(500L);
325+
final DefaultRequestFuture requestFuture = new DefaultRequestFuture(CONNECTION_ID, REQUEST_ID, callback,
326+
trigger);
327+
TimeUnit.MILLISECONDS.sleep(100L);
328+
requestFuture.cancel(true);
329+
assertNull(callback.response);
330+
assertNull(callback.exception);
331+
assertFalse(trigger.isTimeout);
332+
assertTrue(trigger.isCancel);
333+
assertEquals(callback, requestFuture.getRequestCallBack());
334+
}
335+
336+
private class MockFutureTrigger implements DefaultRequestFuture.FutureTrigger {
286337

287338
boolean isTimeout;
288339

340+
boolean isCancel;
341+
342+
@Override
343+
public void defaultTrigger() {
344+
// do nothing
345+
}
346+
289347
@Override
290348
public void triggerOnTimeout() {
291349
isTimeout = true;
292350
}
351+
352+
@Override
353+
public void triggerOnCancel() {
354+
isCancel = true;
355+
}
293356
}
294357

295358
private class MockRequestCallback implements RequestCallBack<Response> {
@@ -331,4 +394,4 @@ public void onException(Throwable e) {
331394
exception = e;
332395
}
333396
}
334-
}
397+
}

‎config/src/main/java/com/alibaba/nacos/config/server/service/repository/extrnal/ExternalConfigInfoPersistServiceImpl.java‎

100644100755
File mode changed.

‎core/src/main/java/com/alibaba/nacos/core/remote/grpc/GrpcConnection.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,12 @@ private DefaultRequestFuture sendRequestInner(Request request, RequestCallBack c
149149
callBack, () -> RpcAckCallbackSynchronizer.clearFuture(getMetaInfo().getConnectionId(), requestId));
150150

151151
RpcAckCallbackSynchronizer.syncCallback(getMetaInfo().getConnectionId(), requestId, defaultPushFuture);
152-
sendRequestNoAck(request);
152+
try {
153+
sendRequestNoAck(request);
154+
} catch (NacosRuntimeException nacosRuntimeException) {
155+
defaultPushFuture.cancel(true);
156+
throw nacosRuntimeException;
157+
}
153158
return defaultPushFuture;
154159
}
155160

0 commit comments

Comments
 (0)