You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by li...@apache.org on 2020/08/28 08:01:36 UTC

[dubbo] branch master updated: remove notification retry task (#6401)

This is an automated email from the ASF dual-hosted git repository.

liujun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/dubbo.git


The following commit(s) were added to refs/heads/master by this push:
     new dc297d0  remove notification retry task (#6401)
dc297d0 is described below

commit dc297d04397df45d5525af94aecb6ee492a600cf
Author: ken.lj <ke...@gmail.com>
AuthorDate: Fri Aug 28 16:00:35 2020 +0800

    remove notification retry task (#6401)
    
    fixes #5961
---
 .../dubbo/registry/support/FailbackRegistry.java   |  4 --
 .../dubbo/registry/retry/FailedNotifiedTask.java   | 67 ----------------------
 .../dubbo/registry/support/FailbackRegistry.java   | 39 +------------
 .../registry/support/FailbackRegistryTest.java     | 31 ----------
 4 files changed, 2 insertions(+), 139 deletions(-)

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 21f4675..4182ea6 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 abstract class FailbackRegistry implements org.apache.dubbo.registry.Regi
         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 43084c6..0000000
--- 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<URL> 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<URL> urls) {
-        if (CollectionUtils.isEmpty(urls)) {
-            return;
-        }
-        this.urls.addAll(urls);
-    }
-
-    public void removeRetryUrl(List<URL> 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 0197b3e..6d82c05 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.timer.HashedWheelTimer;
 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<Holder, FailedUnsubscribedTask> failedUnsubscribed = new ConcurrentHashMap<Holder, FailedUnsubscribedTask>();
 
-    private final ConcurrentMap<Holder, FailedNotifiedTask> failedNotified = new ConcurrentHashMap<Holder, FailedNotifiedTask>();
-
     /**
      * The time in milliseconds the retryExecutor will wait
      */
@@ -93,11 +90,6 @@ public abstract class FailbackRegistry extends AbstractRegistry {
         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 @@ public abstract class FailbackRegistry extends AbstractRegistry {
             f.cancel();
         }
         removeFailedUnsubscribed(url, listener);
-        removeFailedNotified(url, listener);
     }
 
     private void addFailedUnsubscribed(URL url, NotifyListener listener) {
@@ -184,28 +175,6 @@ public abstract class FailbackRegistry extends AbstractRegistry {
         }
     }
 
-    private void addFailedNotified(URL url, NotifyListener listener, List<URL> 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<URL, FailedRegisteredTask> getFailedRegistered() {
         return failedRegistered;
     }
@@ -222,9 +191,6 @@ public abstract class FailbackRegistry extends AbstractRegistry {
         return failedUnsubscribed;
     }
 
-    ConcurrentMap<Holder, FailedNotifiedTask> getFailedNotified() {
-        return failedNotified;
-    }
 
     @Override
     public void register(URL url) {
@@ -397,9 +363,8 @@ public abstract class FailbackRegistry extends AbstractRegistry {
         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 2687956..94c44a0 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 org.junit.jupiter.api.Test;
 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;
@@ -154,36 +153,6 @@ public class FailbackRegistryTest {
     }
 
     @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<URL> 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);
         final AtomicReference<Boolean> notified = new AtomicReference<Boolean>(false);