From c6e0d2a4ccf84c8c0e0d6e03062222ce4ed9a8f6 Mon Sep 17 00:00:00 2001 From: "ken.lj" Date: Fri, 28 Aug 2020 16:00:35 +0800 Subject: [PATCH] remove notification retry task (#6401) fixes #5961 --- .../registry/support/FailbackRegistry.java | 4 -- .../registry/retry/FailedNotifiedTask.java | 67 ------------------- .../registry/support/FailbackRegistry.java | 39 +---------- .../support/FailbackRegistryTest.java | 31 --------- 4 files changed, 2 insertions(+), 139 deletions(-) delete mode 100644 dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/retry/FailedNotifiedTask.java diff --git a/dubbo-compatible/src/main/java/com/alibaba/dubbo/registry/support/FailbackRegistry.java b/dubbo-compatible/src/main/java/com/alibaba/dubbo/registry/support/FailbackRegistry.java index 21f4675e78b..4182ea65c5c 100644 --- a/dubbo-compatible/src/main/java/com/alibaba/dubbo/registry/support/FailbackRegistry.java +++ b/dubbo-compatible/src/main/java/com/alibaba/dubbo/registry/support/FailbackRegistry.java @@ -51,10 +51,6 @@ public void removeFailedUnsubscribedTask(URL url, NotifyListener listener) { failbackRegistry.removeFailedUnsubscribedTask(url.getOriginalURL(), new NotifyListener.ReverseCompatibleNotifyListener(listener)); } - public void removeFailedNotifiedTask(URL url, NotifyListener listener) { - failbackRegistry.removeFailedNotifiedTask(url.getOriginalURL(), new NotifyListener.ReverseCompatibleNotifyListener(listener)); - } - @Override public void register(URL url) { failbackRegistry.register(url.getOriginalURL()); diff --git a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/retry/FailedNotifiedTask.java b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/retry/FailedNotifiedTask.java deleted file mode 100644 index 43084c6558a..00000000000 --- a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/retry/FailedNotifiedTask.java +++ /dev/null @@ -1,67 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.dubbo.registry.retry; - -import org.apache.dubbo.common.URL; -import org.apache.dubbo.common.timer.Timeout; -import org.apache.dubbo.common.utils.CollectionUtils; -import org.apache.dubbo.registry.NotifyListener; -import org.apache.dubbo.registry.support.FailbackRegistry; - -import java.util.List; -import java.util.concurrent.CopyOnWriteArrayList; - -/** - * FailedNotifiedTask - */ -public final class FailedNotifiedTask extends AbstractRetryTask { - - private static final String NAME = "retry notify"; - - private final NotifyListener listener; - - private final List urls = new CopyOnWriteArrayList<>(); - - public FailedNotifiedTask(URL url, NotifyListener listener) { - super(url, null, NAME); - if (listener == null) { - throw new IllegalArgumentException(); - } - this.listener = listener; - } - - public void addUrlToRetry(List urls) { - if (CollectionUtils.isEmpty(urls)) { - return; - } - this.urls.addAll(urls); - } - - public void removeRetryUrl(List urls) { - this.urls.removeAll(urls); - } - - @Override - protected void doRetry(URL url, FailbackRegistry registry, Timeout timeout) { - if (CollectionUtils.isNotEmpty(urls)) { - listener.notify(urls); - urls.clear(); - } - reput(timeout, retryPeriod); - } -} diff --git a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/FailbackRegistry.java b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/FailbackRegistry.java index 0197b3e0c7b..6d82c055f71 100644 --- a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/FailbackRegistry.java +++ b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/FailbackRegistry.java @@ -21,7 +21,6 @@ import org.apache.dubbo.common.utils.CollectionUtils; import org.apache.dubbo.common.utils.NamedThreadFactory; import org.apache.dubbo.registry.NotifyListener; -import org.apache.dubbo.registry.retry.FailedNotifiedTask; import org.apache.dubbo.registry.retry.FailedRegisteredTask; import org.apache.dubbo.registry.retry.FailedSubscribedTask; import org.apache.dubbo.registry.retry.FailedUnregisteredTask; @@ -57,8 +56,6 @@ public abstract class FailbackRegistry extends AbstractRegistry { private final ConcurrentMap failedUnsubscribed = new ConcurrentHashMap(); - private final ConcurrentMap failedNotified = new ConcurrentHashMap(); - /** * The time in milliseconds the retryExecutor will wait */ @@ -93,11 +90,6 @@ public void removeFailedUnsubscribedTask(URL url, NotifyListener listener) { failedUnsubscribed.remove(h); } - public void removeFailedNotifiedTask(URL url, NotifyListener listener) { - Holder h = new Holder(url, listener); - failedNotified.remove(h); - } - private void addFailedRegistered(URL url) { FailedRegisteredTask oldOne = failedRegistered.get(url); if (oldOne != null) { @@ -159,7 +151,6 @@ private void removeFailedSubscribed(URL url, NotifyListener listener) { f.cancel(); } removeFailedUnsubscribed(url, listener); - removeFailedNotified(url, listener); } private void addFailedUnsubscribed(URL url, NotifyListener listener) { @@ -184,28 +175,6 @@ private void removeFailedUnsubscribed(URL url, NotifyListener listener) { } } - private void addFailedNotified(URL url, NotifyListener listener, List urls) { - Holder h = new Holder(url, listener); - FailedNotifiedTask newTask = new FailedNotifiedTask(url, listener); - FailedNotifiedTask f = failedNotified.putIfAbsent(h, newTask); - if (f == null) { - // never has a retry task. then start a new task for retry. - newTask.addUrlToRetry(urls); - retryTimer.newTimeout(newTask, retryPeriod, TimeUnit.MILLISECONDS); - } else { - // just add urls which needs retry. - newTask.addUrlToRetry(urls); - } - } - - private void removeFailedNotified(URL url, NotifyListener listener) { - Holder h = new Holder(url, listener); - FailedNotifiedTask f = failedNotified.remove(h); - if (f != null) { - f.cancel(); - } - } - ConcurrentMap getFailedRegistered() { return failedRegistered; } @@ -222,9 +191,6 @@ ConcurrentMap getFailedUnsubscribed() { return failedUnsubscribed; } - ConcurrentMap getFailedNotified() { - return failedNotified; - } @Override public void register(URL url) { @@ -397,9 +363,8 @@ protected void notify(URL url, NotifyListener listener, List urls) { try { doNotify(url, listener, urls); } catch (Exception t) { - // Record a failed registration request to a failed list, retry regularly - addFailedNotified(url, listener, urls); - logger.error("Failed to notify for subscribe " + url + ", waiting for retry, cause: " + t.getMessage(), t); + // Record a failed registration request to a failed list + logger.error("Failed to notify addresses for subscribe " + url + ", cause: " + t.getMessage(), t); } } diff --git a/dubbo-registry/dubbo-registry-api/src/test/java/org/apache/dubbo/registry/support/FailbackRegistryTest.java b/dubbo-registry/dubbo-registry-api/src/test/java/org/apache/dubbo/registry/support/FailbackRegistryTest.java index 2687956a6f1..94c44a056eb 100644 --- a/dubbo-registry/dubbo-registry-api/src/test/java/org/apache/dubbo/registry/support/FailbackRegistryTest.java +++ b/dubbo-registry/dubbo-registry-api/src/test/java/org/apache/dubbo/registry/support/FailbackRegistryTest.java @@ -27,7 +27,6 @@ import java.util.Arrays; import java.util.List; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import static org.apache.dubbo.registry.Constants.CONSUMER_PROTOCOL; @@ -153,36 +152,6 @@ public void notify(List urls) { assertEquals(true, notified.get()); } - @Test - public void testDoRetry_nofify() throws Exception { - - //Initial value 0 - final AtomicInteger count = new AtomicInteger(0); - - NotifyListener listner = new NotifyListener() { - @Override - public void notify(List urls) { - count.incrementAndGet(); - //The exception is thrown for the first time to see if the back will be called again to incrementAndGet - if (count.get() == 1L) { - throw new RuntimeException("test exception please ignore"); - } - } - }; - registry = new MockRegistry(registryUrl, new CountDownLatch(0)); - registry.subscribe(serviceUrl.setProtocol(CONSUMER_PROTOCOL).addParameters(CollectionUtils.toStringMap("check", "false")), listner); - - assertEquals(1, count.get()); //Make sure that the subscribe call has just been called once count.incrementAndGet after the call is completed - //Wait for the timer. - for (int i = 0; i < trytimes; i++) { - System.out.println("failback notify retry ,times:" + i); - if (count.get() == 2) - break; - Thread.sleep(sleeptime); - } - assertEquals(2, count.get()); - } - @Test public void testRecover() throws Exception { CountDownLatch countDownLatch = new CountDownLatch(4);