-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
xds: add weighted round robin LB policy support #9873
Changes from 3 commits
1f7bbd8
73cf208
12e7928
2727d9b
764c9a9
afa10fb
d4f785d
60af73c
4228f97
cf3d640
c7bcfcd
975eeed
5300199
2458ac9
9cd33b6
23231eb
3d51405
e6886c1
3da127e
cb31730
01a7d1a
6e496da
6dfa072
f786098
0bdce32
3979ef8
0704a69
fc0d3dd
f728281
1ef6221
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -151,9 +151,9 @@ static final class WrrSubchannel extends ForwardingSubchannel { | |
private final TimeProvider timeProvider; | ||
private final OrcaOobReportListener oobListener = this::onLoadReport; | ||
private final OrcaPerRequestReportListener perRpcListener = this::onLoadReport; | ||
volatile long lastUpdated; | ||
volatile long nonEmptySince; | ||
volatile double weight; | ||
private volatile long lastUpdated; | ||
private volatile long nonEmptySince; | ||
private volatile double weight; | ||
private volatile WeightedRoundRobinLoadBalancerConfig config; | ||
|
||
WrrSubchannel(Subchannel delegate, TimeProvider timeProvider) { | ||
|
@@ -217,13 +217,13 @@ protected Subchannel delegate() { | |
final class WeightedRoundRobinPicker extends ReadyPicker { | ||
private final List<Subchannel> list; | ||
private final AtomicReference<EdfScheduler> schedulerRef; | ||
YifeiZhuang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private volatile boolean rrMode = false; | ||
private volatile boolean rrMode = true; | ||
YifeiZhuang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
WeightedRoundRobinPicker(List<Subchannel> list, int startIndex) { | ||
super(list, startIndex); | ||
super(checkNotNull(list, "list"), startIndex); | ||
Preconditions.checkArgument(!list.isEmpty(), "empty list"); | ||
this.list = list; | ||
this.schedulerRef = new AtomicReference<>(new EdfScheduler()); | ||
this.schedulerRef = new AtomicReference<>(new EdfScheduler(list.size())); | ||
YifeiZhuang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
updateWeight(); | ||
} | ||
|
||
|
@@ -245,7 +245,7 @@ public PickResult pickSubchannel(PickSubchannelArgs args) { | |
} | ||
|
||
private void updateWeight() { | ||
EdfScheduler scheduler = new EdfScheduler(); | ||
EdfScheduler scheduler = new EdfScheduler(list.size()); | ||
int weightedChannelCount = 0; | ||
double avgWeight = 0; | ||
for (Subchannel value : list) { | ||
|
@@ -255,22 +255,24 @@ private void updateWeight() { | |
weightedChannelCount++; | ||
} | ||
} | ||
rrMode = weightedChannelCount < 2; | ||
if (rrMode) { | ||
if (weightedChannelCount < 2) { | ||
rrMode = true; | ||
return; | ||
} | ||
avgWeight /= 1.0 * weightedChannelCount; | ||
for (int i = 0; i < list.size(); i++) { | ||
WrrSubchannel subchannel = (WrrSubchannel) list.get(i); | ||
double newWeight = subchannel.getWeight(); | ||
scheduler.add(i, newWeight > 0 ? newWeight : avgWeight); | ||
} | ||
schedulerRef.set(scheduler); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because you are completely replacing the scheduler each time you update weights, if something has a large weight and something else has a minimum weight the second one might never get used. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Absolutely true. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Randomizing the scheduler each creation should prevent that. Worst-case, if a schedule is used for only a single pick after being created, then that is the same as a WRR implementation that has weighted ranges for each choice and uses a random number to choose (same approach as weighted_target). The code right now does not seem to randomize the initial scheduler state, but it will need to before de-experimentalizing. |
||
rrMode = false; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return MoreObjects.toStringHelper(WeightedRoundRobinPicker.class) | ||
.add("list", list).toString(); | ||
.add("list", list).add("rrMode", rrMode).toString(); | ||
} | ||
|
||
@VisibleForTesting | ||
|
@@ -294,29 +296,27 @@ public boolean isEquivalentTo(RoundRobinPicker picker) { | |
* The earliest deadline first implementation in which each object is | ||
* chosen deterministically and periodically with frequency proportional to its weight. | ||
* | ||
* <p>Specifically, each object added to chooser is given a period equal to the multiplicative | ||
* inverse of its weight. The place of each object in its period is tracked, and each call to | ||
* choose returns the object with the least remaining time in its period (1/weight). | ||
* <p>Specifically, each object added to chooser is given a deadline equal to the multiplicative | ||
YifeiZhuang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* inverse of its weight. The place of each object in its deadline is tracked, and each call to | ||
* choose returns the object with the least remaining time in its deadline (1/weight). | ||
* (Ties are broken by the order in which the children were added to the chooser.) | ||
* For example, if items A and B are added | ||
* with weights 0.5 and 0.2, successive chooses return: | ||
* For example, if items A and B are added with weights 0.5 and 0.2, successive chooses return: | ||
* | ||
* <ul> | ||
* <li>In the first call, the remaining periods are A=2 (1/0.5) and B=5 (1/0.2), so A is | ||
* returned. The period of A (as it was picked), is substracted from periods of all other | ||
* objects. | ||
* <li>Next, the remaining periods are A=2 and B=3, so A is returned. The period of A (2) is | ||
* substracted from all other objects (B=1) and A is re-added with A=2. | ||
* <li>Remaining periods are A=2 and B=1, so B is returned. The period of B (1) is substracted | ||
* from all other objects (A=1) and B is re-added with B=5. | ||
* <li>Remaining periods are A=1 and B=5, so A is returned. The period of A (1) is substracted | ||
* from all other objects (B=4) and A is re-added with A=2. | ||
* <li>Remaining periods are A=2 and B=4, so A is returned. The period of A (2) is substracted | ||
* from all other objects (B=2) and A is re-added with A=2. | ||
* <li>Remaining periods are A=2 and B=2, so A is returned. The period of A (2) is substracted | ||
* from all other objects (B=0) and A is re-added with A=2. | ||
* <li>Remaining periods are A=2 and B=0, so B is returned. The period of B (0) is substracted | ||
* from all other objects (A=2) and B is re-added with B=5. | ||
* <li>In the first call, the deadlines are A=2 (1/0.5) and B=5 (1/0.2), so A is returned. | ||
* The deadline of A is updated to 4. | ||
* <li>Next, the remaining deadlines are A=4 and B=5, so A is returned. The deadline of A (2) is | ||
* updated to A=6. | ||
* <li>Remaining deadlines are A=6 and B=5, so B is returned. The deadline of B is updated with | ||
* with B=10. | ||
* <li>Remaining deadlines are A=6 and B=10, so A is returned. The deadline of A is updated with | ||
* A=8. | ||
* <li>Remaining deadlines are A=8 and B=10, so A is returned. The deadline of A is updated with | ||
* A=10. | ||
* <li>Remaining deadlines are A=10 and B=10, so A is returned. The deadline of A is updated | ||
* with A=12. | ||
* <li>Remaining deadlines are A=12 and B=10, so B is returned. The deadline of B is updated | ||
* with B=15. | ||
* <li>etc. | ||
* </ul> | ||
* | ||
|
@@ -333,13 +333,6 @@ public boolean isEquivalentTo(RoundRobinPicker picker) { | |
private static final class EdfScheduler { | ||
private final PriorityQueue<ObjectState> prioQueue; | ||
|
||
/** | ||
* Upon every pick() the "virtual time" is advanced closer to the period of next items. | ||
* Here we have an explicit "virtualTimeNow", which will be added to the period of all newly | ||
* scheduled objects (virtualTimeNow + period). | ||
*/ | ||
private double virtualTimeNow = 0.0; | ||
|
||
/** | ||
* Weights below this value will be logged and upped to this minimum weight. | ||
YifeiZhuang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*/ | ||
|
@@ -351,8 +344,8 @@ private static final class EdfScheduler { | |
* Use the item's deadline as the order in the priority queue. If the deadlines are the same, | ||
* use the index. Index should be unique. | ||
*/ | ||
EdfScheduler() { | ||
this.prioQueue = new PriorityQueue<ObjectState>(10, (o1, o2) -> { | ||
EdfScheduler(int initialCapacity) { | ||
this.prioQueue = new PriorityQueue<ObjectState>(initialCapacity, (o1, o2) -> { | ||
if (o1.deadline == o2.deadline) { | ||
return o1.index - o2.index; | ||
} else if (o1.deadline < o2.deadline) { | ||
YifeiZhuang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
@@ -372,7 +365,7 @@ private static final class EdfScheduler { | |
void add(int index, double weight) { | ||
checkArgument(weight > 0.0, "Weights need to be positive."); | ||
ObjectState state = new ObjectState(Math.max(weight, MINIMUM_WEIGHT), index); | ||
state.deadline = virtualTimeNow + 1 / state.weight; | ||
state.deadline = 1 / state.weight; | ||
YifeiZhuang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
prioQueue.add(state); | ||
} | ||
|
||
|
@@ -382,10 +375,7 @@ void add(int index, double weight) { | |
int pick() { | ||
synchronized (lock) { | ||
ObjectState minObject = prioQueue.remove(); | ||
// Simulate advancing in time by setting the current time to the period of the nearest item | ||
// on the "time horizon". | ||
virtualTimeNow = minObject.deadline; | ||
minObject.deadline = virtualTimeNow + (1.0 / minObject.weight); | ||
minObject.deadline += 1.0 / minObject.weight; | ||
prioQueue.add(minObject); | ||
return minObject.index; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this
volatile
? It appears to only be accessed in the synchronization context.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh it appears so.It can be used by pick() from
SubchannelStateListener
from transport thread, or here in the LB thread in weightUpdateTask or acceptResolvedAddresses, all in sync context.