对于FailedNotifiedTask类doRetry方法的疑问

[yoozoom]

• I have searched the issues of this repository and believe that this is not a duplicate.
• I have checked the FAQ of this repository and believe that this is not a duplicate.

Environment

• Dubbo version: 2.7.6
• Operating System version: win10
• Java version: 1.8

Steps to reproduce this issue

对于FailedNotifiedTask类doRetry方法的疑问

1. 为什么要无差别reput这个任务，使他执行3次（默认重试次数）
2. 为什么调用listener.notify(urls)来进行重试。正常逻辑是调用FailbackRegistry.notify，再调用listener.notify。FailbackRegistry.notify和listener.notify中间相差很多逻辑，为什么在重试任务中直接使用listener.notify而忽略了相差的逻辑

Pls. provide [GitHub address] to reproduce this issue.

Expected Result

@Override
protected void doRetry(URL url, FailbackRegistry registry, Timeout timeout) {
    if (CollectionUtils.isNotEmpty(urls)) {
        registry.doNotify(urls);
        urls.clear();
    }
    //reput(timeout, retryPeriod);
}

Actual Result

@Override
protected void doRetry(URL url, FailbackRegistry registry, Timeout timeout) {
    if (CollectionUtils.isNotEmpty(urls)) {
        listener.notify(urls);
        urls.clear();
    }
    reput(timeout, retryPeriod);
}

理解不深，烦请解答。

[wavesZh]

@yoozoom

1. 注意 FailedNotifiedTask#addUrlToRetry 方法以及 doRetry 方法中对于 urls 的处理。我感觉 reput 意义是为了避免重新new task，如果后面provider再次变更，可以直接复用这个task，在 FailbackRegistry#addFailedNotified 中可以看出。
2. 直接使用 listener.notify 感觉是有缺陷的。

[yoozoom]

@wavesZh

1. FailedNotifiedTask#addUrlToRetry和doRetry的reput并无矛盾之处。无论url结果如何，都会无差别reput当然任务，导致任务执行3次。这种执行3次的任务是否可以优化。因为FailedNotifiedTask的父类已经有了对错误进行重试的reput逻辑，子类一般不需要显示提交reput。

另外FailedNotifiedTask#addUrlToRetry的调用点FailbackRegistry#addFailedNotified

    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);
        }
    }

在failedNotified有值，f != null的分支中，给新new出来的newTask添加url后，对newTask没有提交给timer，视乎也有问题。是否应该为:

        } else {
            // just add urls which needs retry.
            f.addUrlToRetry(urls);
        }

2. 还请指明直接使用 listener.notify 的缺陷。

感谢回复。

[wavesZh]

@yoozoom

1. addFailedNotified 方法的逻辑的确有些问题。应该是要用 f.addUrlToRetry(urls), cancel 后 重新提交任务至timer。无差别reput看起来的确太多意义。
2. 就如你所说的，少了很多处理逻辑。如 AbstractRegistry#saveProperties 没有执行。不过看提交历史，之前很早重试也是只调用 listener.notify，这让人费解。
   也许你可以来个pr优化下。

[yoozoom]

@wavesZh 感谢回复。
目前只是看notify代码时有些疑惑，怕作者对这块代码有什么特殊处理，所以提出来讨论下。