spring-retry icon indicating copy to clipboard operation
spring-retry copied to clipboard

default implementation of @Backoff() should not use Thread.sleep

Open jonnytest1 opened this issue 2 years ago • 7 comments
trafficstars

we used the @Retryable annotation in a project recently and added a backoff of like one day to our decently long running job , due to the implementation with Thread.sleep this meant that no other scheduled jobs would trigger , which i tihnk isnt generally an expected behaviour , instead it should

  1. be dcoumented on @Retryableand its backoff property ,@Backoff and its delay property (in all caps if need be) that this sleeps the Thread
  2. the default implementation should be
      TimerTask task = new TimerTask() {
        public void run() {
            System.out.println("Task performed on: " + new Date() + "n" +
              "Thread's name: " + Thread.currentThread().getName());
        }
    };
    Timer timer = new Timer("Timer");
    
    long delay = 1000L;
    timer.schedule(task, delay);
}

or another similar nonblocking implemenation

jonnytest1 avatar Oct 17 '23 14:10 jonnytest1

I agree with you jonnytest1. could you please assign this to me, There is my PR for fixing this issue , If it's not OK please comment and provide more information for fixing it.

hakimrabet avatar May 23 '24 23:05 hakimrabet

i do not know how to assign issues to someone 🤔

jonnytest1 avatar May 24 '24 12:05 jonnytest1

i do not know how to assign issues to someone

The issue can be assigned only to Spring team member. However that should not be a stopper for community contribution.

We will look into your PR eventually, however I'm a bit skeptic with non-blocking by default. The retry pattern supposed to be transparent for API consumer. So, you just call the method and expect some logic to be done. The fact that it is advised with a retry must not bother you. And if you indeed configured very long back-off, it is expected to have call stack blocked. We cannot release that thread since we don't have a result from business method execution.

What you probably what is something like async retry: https://github.com/spring-projects/spring-retry/pull/176. There we would schedule a new CompletebleFuture which would be fulfilled eventually without blocking the current thread. But as you see it fully involves everything to be async: we just cannot go async with a regular blocking method.

artembilan avatar May 24 '24 15:05 artembilan

indeed you are right, however it's not for reactive project instead for regular project, main idea of that is to release the caller thread When scheduler running thread going to sleep the other scheduler waiting for the thread going to be released

hakimrabet avatar May 24 '24 21:05 hakimrabet

Thread.sleep() will unmount virtual thread from its carrier platform thread, you should try virtual thread from JDK 21+.

quaff avatar Aug 21 '24 08:08 quaff

@quaff ,

your sentence does not compile in my head. Virtual threads are really from Java 21. And yes, they are unmounted from platform thread. But the point of Thread.sleep() is still to block the current thread, which is no difference for us if it is virtual or not.

artembilan avatar Aug 21 '24 14:08 artembilan

But the point of Thread.sleep() is still to block the current thread, which is no difference for us if it is virtual or not.

That's expected behavior, we need delay here but do not waste CPU resource. My point is Thread.sleep() is not terrible now.

quaff avatar Aug 22 '24 01:08 quaff