ConcurrentModificationMiddleware does not retry DELETE HTTP requests

[ZanochkynYehor]

SDK version: 19.7.3, but the behaviour is the same on the latest version as well

We're using the ConcurrentModificationMiddleware to retry the requests in case of a 409 concurrent modification response. It works great for POST HTTP requests, but it doesn't work at all for DELETE HTTP requests. For DELETE requests, when our service receives a 409 concurrent modification response, it just does nothing for a couple of seconds (waits for all the delays between the retries) without doing actual retry requests.

Is it possible to update this middleware to also work for DELETE requests?

[jenschude]

The SDK should not retry a destructive action by updating the version number. For this use case the RetryHandler is available which can update the request.

But you are still correct, that it the concurrentModification middleware will refire the requests but they still fail.

In the linked PR the retry will be aborted for delete requests.

[ZanochkynYehor]

Sounds good, thank you

[ZanochkynYehor]

Hey @jenschude. We've updated the SDK to version 19.10.0 and noticed a strange behavior. It looks like sometimes SDK thinks it has to retry the request, waits for the initial delay, and only after that aborts the retries. At the same time, in another request, the SDK doesn't even attempt to retry the request. Below is a screenshot of 2 different traces of the same exact request, where we can see that the first request finishes almost immediately after the 409 CT response is received, while the second one additionally waits for ~80 ms on average after the 409 response:

Do you have an idea why this could be happening?

[ZanochkynYehor]

We have a custom logging middleware, and we have a log from the second trace that says that the DELETE request took 110ms, but from the trace we can see that it actually took only 29.3ms. Our logging middleware is usually pretty accurate in terms of time spent, so this makes me think it's indeed something wrong inside the SDK.

[jenschude]

More interesting is if the delete is being retried or is it just an delay in the execution? Cause these delays could be still explained by pressure on the ForkJoinPools and when a task can be executed

[ZanochkynYehor]

From my perspective, it looks like a delay between the initial request and the retry. We've never seen cases when our service doesn't have enough resources to execute tasks on time

[jenschude]

Also to clarify what happened before the change:

The Middleware captured ConcurrentModification exceptions and modified the request body for Post requests with the actual version returned by the API so that it can be retried.

As the delete request doesn't have a body nothing would be changed and the request is being retried as is, which resulted in a new ConcurrentModification exception.

The change now added a condition to the RetryHandler to abort if it's a concurrentmodification exception and a delete request. So that there will be no retry happening for deletes.

[ZanochkynYehor]

I'm wondering if this RetryPolicy.abortIf is checked only during the retry attempt?

[jenschude]

From the failsafe documentation:

abortIf -> Specifies that retries should be aborted if the resultPredicate matches the result. Predicate is not invoked when the operation fails. Any exception thrown from the resultPredicate is treated as a false result.

[ZanochkynYehor]

After some investigation, I believe we see this additional delay only when there is no load on the server. I don't see obvious delays when the server is under load. We'll propagate this change to production and monitor it there. I'll come back if I find anything. Thank you