Skip to content
This repository has been archived by the owner on Jul 25, 2020. It is now read-only.

JCLOUDS-460: Add jitter to avoid thundering herd #286

Closed
wants to merge 1 commit into from
Closed

JCLOUDS-460: Add jitter to avoid thundering herd #286

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions core/src/main/java/org/jclouds/http/handlers/BackoffLimitedRetryHandler.java
View file
Open in desktop
Expand Up @@ -19,6 +19,7 @@
import static org.jclouds.http.HttpUtils.releasePayload;

import java.io.IOException;
import java.util.Random;

import javax.annotation.Resource;
import javax.inject.Named;
Expand Down Expand Up @@ -126,6 +127,9 @@ public void imposeBackoffExponentialDelay(long period, int pow, int failureCount
public void imposeBackoffExponentialDelay(long period, long maxPeriod, int pow, int failureCount, int max,
String commandDescription) {
long delayMs = (long) (period * Math.pow(failureCount, pow));
// Add random delay to avoid thundering herd problem when multiple
// simultaneous failed requests retry after sleeping for the same delay.
delayMs += new Random().nextInt((int) (delayMs / 10));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be very small if delayMs happens to be small. Is that intentional?

Also, can we use a shared Random here - they should be threadsafe. That's not as random as using a new one every time, but for this application that should be fine?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delayMs defaults to a small value, 50 ms, although scales up to 500 ms. 10% jitter may be suboptimal; I chose this value to agree with our existing tests.

Java 6 Javadoc does not guarantee thread-safety:

http://docs.oracle.com/javase/6/docs/api/java/util/Random.html

Although StackOverflow and the Java bug tracker suggest that it is:

http://stackoverflow.com/questions/5819638/is-random-class-thread-safe
http://bugs.java.com/bugdatabase/view_bug.do?bug_id=6362070

Prefer to keep this simple if possible since we only see this on the uncommon failure path.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we only see this on the uncommon failure path.

Ah, OK. Then let's keep as-is. Just wondering if we want to change the jitter to min(5ms, delay * 0.1) or so?

delayMs = delayMs > maxPeriod ? maxPeriod : delayMs;
logger.debug("Retry %d/%d: delaying for %d ms: %s", failureCount, max, delayMs, commandDescription);
try {
Expand Down