implemented and tested static stride scheduler for weighted round robin load balancing policy

[tonyjongyoonan]

The core of the Weighted Round Robin load balancer policy on the client side is a stride scheduler originally implemented by an EDFScheduler. However, the mutex lock required by the EDFScheduler has been a frequent source of thread contention at high request rates and a block on other cost saving efforts.

The Static Stride Scheduler is a generator of a practically equivalent sequence of picks as the current EDFScheduler. It removes the need for a priority queue (and thus a lock) and improves latency at high request rates.

Weighted Round Robin LB Policy

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: tonyjongyoonan / name: Tony An (b5cf7b0, 44a5158, 32973d4, 4bde79d, d99d51a, 13a9fd8, 3d9e625, 2b054d3, 4addda3, 4a820a7, dc7960a, acd3425, ba1a0b7, 9f4a60d, e11e542, c76cbe5, 903d2ac, d5a0629, 5982115, 88a8e48, f9cae20, dba0778, 46a463e, 142b499, 5523337, 365ba8d, 442bea8, 2a0e489, 1731862, 5e5127f, f0421d2, ec46fe3, 8ddf284, 347b46f, 4a4762e, f526bf6, 21ceb85, 03de3f9, e2eb7f9, 4072907, f749e52, be21c23)

[ejona86]

You can continue pushing to your branch and update the PR as you go.

[ejona86]

This should be an AtomicInteger. If you haven't seen https://github.com/grpc/proposal/blob/master/A58-client-side-weighted-round-robin-lb-policy.md#earliest-deadline-first-edf-scheduler , there's a few things there. We don't have to do the identical approach, but we'd avoid doing things differently when unnecessary.

lambda () -> uint32 next_seq_fn is "a function you call to get an int32. The equivalent of that in Java is AtomicInteger.getAndIncrement(). One important part of that function in the design is that it is shared across scheduler instances. If you re-create the scheduler for the same input data, you get the same results, so the code doesn't have to worry about poor weighting if it is re-created frequently. Since we didn't have that sort of state previously, the old scheduler had to randomize itself.

[YifeiZhuang]

After the seq reaches Integer.MAX_VALUE , it rolls over and this becomes -1. The seq should be non negative.

The staticScheduler lives in the picker, and there might be multiple instances of picker in the system, so ideally the seq should be global among all the pickers. Alternatively, we need to randomize the weight during the static scheduler construction time.

[tonyjongyoonan]

Should be fixed.

[ejona86]

Since you are storing 32 bits, instead of AtomicLong, use AtomicInteger. Then, you just need to cast up to long as you read it out. That can be done with Integer.toUnsignedLong() or seq & UINT32_MAX. Masking instead of modulus makes it easier to avoid negative results. Then, instead of getAndUpdate(), just use getAndIncrement(). So this becomes:

private final AtomicInteger sequence;

private long nextSequence() {
    return Integer.toUnsignedLong(sequence.getAndIncrement());
}

Note that I am paying attention to & UINT32_MAX being different from % UINT32_MAX. But we don't care about the difference here.

[tonyjongyoonan]

I was under the impression that AtomicInteger was signed, which would mean that it cannot support the max value for an unsigned 32 bit integer. Is this not the case?

[ejona86]

It still has 32 bits and addition (the increment) is the same for signed and unsigned numbers. So the bits are what we'd hope they would be, but Java just won't treat them right in math. When casting to a larger integer size, then only difference between signed and unsigned is whether you sign extend: whether you fill the new bits on the left with 0s or 1s. For unsigned, you always use 0s. For signed, you copy the sign bit to the upper bits. So if you want unsigned conversion but only have signed, you do a signed conversion and then force the top bits to be 0s with the bitwise AND.

We could use AtomicLong and mask out the lower bits, but if we are masking the bits then AtomicInteger is just as good.

[YifeiZhuang]

similarly this should be compared to 0

[ejona86]

We can do it in a later PR (by you or someone else), but we will want to replace this Random with AtomicInteger sequence. The only reason to have the random now is because we couldn't carry state between re-creations of the scheduler. But the only mutable state now is the integer, so we can follow the gRFC.

[YifeiZhuang]

This (L335-L337) is describing the priority queue implementation of EDF and is no longer the case here, but sounds like it is how current implementation is.

[YifeiZhuang]

LGTM.
Please get @ejona86 approval then merge.
Please consult @temawi to watch for interop test dashboard after merge.

[ejona86]

I see why some of that stuff was removed, but the comment now has the same problem an earlier version of this code has: it name-drops EDF without actually saying how it is mapped to a scheduling problem. Let's just replace this line with "in which each object's deadline is the multiplicative inverse of the object's weight." I think that is the most important part of the mapping.

[ejona86]

Annotate with com.google.common.annotations.VisibleForTesting