loopingcall.RetryDecorator doesn't drop _retry_count on successful invocation of a decorated method

Bug #1718635 reported by Renat Akhmerov on 2017-09-21
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
oslo.service
High
Unassigned

Bug Description

I'm not sure if this is a bug or it's made by design.

The semantics of loopingcall.RetryDecorator is now the following: no matter how many times we call a decorated method, if the decorator has to retry the method it will be counting ALL retries ever made internally, not only those made within one user call. Hence once the number of retries reaches max_retry_count param it stops retrying even if we call a decorated method again explicitly.

To illustrate it:

@loopingcall.RetryDecorator(max_retry_count=100, inc_sleep_time=0, exceptions=MyExceptionClass)
def my_method():
   ....
   raises MyExceptionClass # once in a while
   ....

For example, I called my_method once but it was retried by the decorator 90 times because it was raising MyExceptionClass instance 90 times. If I call my_method again then, effectively, max retry count will be 10 because it will be use the same internal _retry_count counter as the previous time.

Seems like, RetryDecorator should drop _retry_count when a decorated method is invoked successfully.

Renat Akhmerov (rakhmerov) wrote :

If it's a bug then there's one more consequence: it's not thread-safe. If we're calling a decorated method from many parallel threads they'll be sharing the counter field.

Ben Nemec (bnemec) wrote :

This definitely seems wrong. I think we could fix the retry_count by resetting it each call, but as you note that wouldn't be threadsafe. Looking at this code, it might be a little tricky to fix properly because of how the decorator interacts with the loopingcall class. It's something we need to address though.

Changed in oslo.service:
status: New → Confirmed
importance: Undecided → High
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers