Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spring Retry Why not use @ Aspect annotation? #358

Closed
whpgit opened this issue Jun 6, 2023 · 6 comments
Closed

Spring Retry Why not use @ Aspect annotation? #358

whpgit opened this issue Jun 6, 2023 · 6 comments
Labels

Comments

@whpgit
Copy link

whpgit commented Jun 6, 2023

What are the main reasons for not using @ Aspect? What I have found so far is that @ Aspect annotations can only perform some operations before and after method calls, and cannot achieve automatic retry functionality.
At present, I want to write a component myself and take these into account
Thank you!

@artembilan
Copy link
Member

Sorry, the question is not clear.

We probably indeed can do an @Aspect and @Around advice method.
But then we need to implement a retry logic ourselves around that Object retVal = pjp.proceed();.

Here is a respective documentation: https://docs.spring.io/spring-framework/reference/core/aop/ataspectj/advice.html

Well, we probably can use a RetryTemplate.execute() around that pjp.proceed(), but is it really a goal we would pursue to make use of those AspectJ annotations where it would feel like an overhead and dedicated Retry API is much better to use.

@whpgit
Copy link
Author

whpgit commented Jun 7, 2023

Firstly, thank you for your reply. Perhaps the question I asked was not expressed clearly, but you have already answered what I wanted, which is exactly my guess.

At present, I have also discovered some areas that can be improved, such as:

  1. Retries with empty result values

  2. @recover annotation

The first input parameter must be a Throwable exception, and the return value of the method must be the same as the return value of @ Retryable. It must be in the same class as @ Retryable, and configuring multiple will only take effect on one, and so on

  1. Consider supporting asynchronous retries, and returning results can be achieved using a callback listener

Finally, thank you very much for your reply. My problem has been resolved.

@artembilan
Copy link
Member

Thank you for confirmation that we are on the same page with your original request.
Now I have doubts in your next points.
It might be better if you share some potential code you are looking for.

And here is some discussion about an async retry: #81

@whpgit
Copy link
Author

whpgit commented Jun 8, 2023

I really enjoyed communicating with you. Just now, I wrote a simple plan for asynchronous retry, which may not have much to offer.

Design concept:

  1. Need Thread pool to submit tasks

  2. Retrying callback interface requires developing incoming

  3. In terms of performance, the pressure on the Thread pool needs to be considered

  4. Monitoring

  5. Stacking cleaning

  6. Retrying the disaster brought by the storm

The code is as follows:

`package com.example.myretry;

import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.context.annotation.Bean;
import org.springframework.stereotype.Component;

import javax.annotation.Resource;
import java.util.concurrent.*;
import java.util.concurrent.atomic.AtomicInteger;

@component
@slf4j
public class AsyncRetryDemo {

@Resource
@Qualifier("retryThreadPoolExecutor")
ThreadPoolExecutor retryThreadPoolExecutor;


public final static int DEFAULT_MAX_ATTEMPTS = 3;


@Bean
public ThreadPoolExecutor retryThreadPoolExecutor() {
    LinkedBlockingQueue<Runnable> asyncRetryThreadPoolQueue = new LinkedBlockingQueue<>(50000);

    return new ThreadPoolExecutor(
            Runtime.getRuntime().availableProcessors(),
            Runtime.getRuntime().availableProcessors(),
            1000 * 60,
            TimeUnit.MILLISECONDS,
            asyncRetryThreadPoolQueue,
            new ThreadFactory() {
                private AtomicInteger threadIndex = new AtomicInteger(0);

                @Override
                public Thread newThread(Runnable r) {
                    return new Thread(r, "AsyncRetryExecutor_" + this.threadIndex.incrementAndGet());
                }
            });
}


public void execute(SendCallback<String> sendCallback) {
    retryThreadPoolExecutor.execute(() -> {
        retry(sendCallback);
    });
}


public void retry(SendCallback<String> sendCallback) {
    int curr = 1;
    while (canRetry(curr)) {
        curr++;
        try {
            int i = 1 / 0;
        } catch (Exception e) {
            log.error("retry exception ", e);
            if (isExhausted(curr)) {
                sendCallback.onException(e);
            }
            continue;
        }

        RetryResult<String> successResult = new RetryResult<>();
        successResult.setData("success");
        sendCallback.onSuccess(successResult);
        break;
    }


}

private boolean canRetry(int curr) {
    return DEFAULT_MAX_ATTEMPTS >= curr;
}

private boolean isExhausted(int curr) {
    return DEFAULT_MAX_ATTEMPTS <= curr;
}


public interface SendCallback<T> {
    void onSuccess(final RetryResult<T> sendResult);
    void onException(final Throwable e);
}

public static class RetryResult<T> {

    private T data;

    public T getData() {
        return data;
    }

    public void setData(T data) {
        this.data = data;
    }
}

}

`

@artembilan
Copy link
Member

I think the original question was answered and everything else goes out of context already.

Closing as Works as Designed.

@artembilan artembilan closed this as not planned Won't fix, can't repro, duplicate, stale Jun 8, 2023
@whpgit
Copy link
Author

whpgit commented Jun 8, 2023

Okay, thank you very much for your reply. This issue has been resolved and completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants