diff --git a/staging/src/k8s.io/apiserver/pkg/util/shufflesharding/shufflesharding.go b/staging/src/k8s.io/apiserver/pkg/util/shufflesharding/shufflesharding.go index 2767b2f0b87..6ef4ed89002 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/shufflesharding/shufflesharding.go +++ b/staging/src/k8s.io/apiserver/pkg/util/shufflesharding/shufflesharding.go @@ -17,117 +17,91 @@ limitations under the License. package shufflesharding import ( - "errors" "fmt" "math" - "strings" ) -const maxHashBits = 60 +// MaxHashBits is the max bit length which can be used from hash value. +// If we use all bits of hash value, the critical(last) card shuffled by +// Dealer will be uneven to 2:3 (first half:second half) at most, +// in order to reduce this unevenness to 32:33, we set MaxHashBits to 60 here. +const MaxHashBits = 60 -// ValidateParameters finds errors in the parameters for shuffle -// sharding. Returns a slice for which `len()` is 0 if and only if -// there are no errors. The entropy requirement is evaluated in a -// fast but approximate way: bits(deckSize^handSize). -func ValidateParameters(deckSize, handSize int) (errs []string) { - if handSize <= 0 { - errs = append(errs, "handSize is not positive") - } - if deckSize <= 0 { - errs = append(errs, "deckSize is not positive") - } - if len(errs) > 0 { - return - } - if handSize > deckSize { - return []string{"handSize is greater than deckSize"} - } - if math.Log2(float64(deckSize))*float64(handSize) > maxHashBits { - return []string{fmt.Sprintf("more than %d bits of entropy required", maxHashBits)} - } - return +// RequiredEntropyBits makes a quick and slightly conservative estimate of the number +// of bits of hash value that are consumed in shuffle sharding a deck of the given size +// to a hand of the given size. The result is meaningful only if +// 1 <= handSize <= deckSize <= 1<<26. +func RequiredEntropyBits(deckSize, handSize int) int { + return int(math.Ceil(math.Log2(float64(deckSize)) * float64(handSize))) } -// ShuffleAndDeal can shuffle a hash value to handSize-quantity and non-redundant -// indices of decks, with the pick function, we can get the optimal deck index -// Eg. From deckSize=128, handSize=8, we can get an index array [12 14 73 18 119 51 117 26], -// then pick function will choose the optimal index from these -// Algorithm: https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/20190228-priority-and-fairness.md#queue-assignment-proof-of-concept -func ShuffleAndDeal(hashValue uint64, deckSize, handSize int, pick func(int)) { - remainders := make([]int, handSize) +// Dealer contains some necessary parameters and provides some methods for shuffle sharding. +// Dealer is thread-safe. +type Dealer struct { + deckSize int + handSize int +} - for i := 0; i < handSize; i++ { - hashValueNext := hashValue / uint64(deckSize-i) - remainders[i] = int(hashValue - uint64(deckSize-i)*hashValueNext) +// NewDealer will create a Dealer with the given deckSize and handSize, will return error when +// deckSize or handSize is invalid as below. +// 1. deckSize or handSize is not positive +// 2. handSize is greater than deckSize +// 3. deckSize is impractically large (greater than 1<<26) +// 4. required entropy bits of deckSize and handSize is greater than MaxHashBits +func NewDealer(deckSize, handSize int) (*Dealer, error) { + if deckSize <= 0 || handSize <= 0 { + return nil, fmt.Errorf("deckSize %d or handSize %d is not positive", deckSize, handSize) + } + if handSize > deckSize { + return nil, fmt.Errorf("handSize %d is greater than deckSize %d", handSize, deckSize) + } + if deckSize > 1<<26 { + return nil, fmt.Errorf("deckSize %d is impractically large", deckSize) + } + if RequiredEntropyBits(deckSize, handSize) > MaxHashBits { + return nil, fmt.Errorf("required entropy bits of deckSize %d and handSize %d is greater than %d", deckSize, handSize, MaxHashBits) + } + + return &Dealer{ + deckSize: deckSize, + handSize: handSize, + }, nil +} + +// Deal shuffles a card deck and deals a hand of cards, using the given hashValue as the source of entropy. +// The deck size and hand size are properties of the Dealer. +// This function synchronously makes sequential calls to pick, one for each dealt card. +// Each card is identified by an integer in the range [0, deckSize). +// For example, for deckSize=128 and handSize=4 this function might call pick(14); pick(73); pick(119); pick(26). +func (d *Dealer) Deal(hashValue uint64, pick func(int)) { + // 15 is the largest possible value of handSize + var remainders [15]int + + for i := 0; i < d.handSize; i++ { + hashValueNext := hashValue / uint64(d.deckSize-i) + remainders[i] = int(hashValue - uint64(d.deckSize-i)*hashValueNext) hashValue = hashValueNext } - for i := 0; i < handSize; i++ { - candidate := remainders[i] + for i := 0; i < d.handSize; i++ { + card := remainders[i] for j := i; j > 0; j-- { - if candidate >= remainders[j-1] { - candidate++ + if card >= remainders[j-1] { + card++ } } - pick(candidate) + pick(card) } } -// ShuffleAndDealWithValidation will do validation before ShuffleAndDeal -func ShuffleAndDealWithValidation(hashValue uint64, deckSize, handSize int, pick func(int)) error { - if errs := ValidateParameters(deckSize, handSize); len(errs) > 0 { - return errors.New(strings.Join(errs, ";")) - } - - ShuffleAndDeal(hashValue, deckSize, handSize, pick) - return nil -} - -// ShuffleAndDealToSlice will use specific pick function to return slices of indices -// after ShuffleAndDeal -func ShuffleAndDealToSlice(hashValue uint64, deckSize, handSize int) []int { - var ( - candidates = make([]int, handSize) - idx = 0 - ) - - pickToSlices := func(can int) { - candidates[idx] = int(can) - idx++ - } - - ShuffleAndDeal(hashValue, deckSize, handSize, pickToSlices) - - return candidates -} - -// ShuffleAndDealIntoHand shuffles a deck of the given size by the -// given hash value and deals cards into the given slice. The virtue -// of this function compared to ShuffleAndDealToSlice is that the -// caller provides the storage for the hand. -func ShuffleAndDealIntoHand(hashValue uint64, deckSize int, hand []int) { - handSize := len(hand) - var idx int - ShuffleAndDeal(hashValue, deckSize, handSize, func(card int) { - hand[idx] = int(card) - idx++ - }) -} - -// ShuffleAndDealToSliceWithValidation will do validation before ShuffleAndDealToSlice -func ShuffleAndDealToSliceWithValidation(hashValue uint64, deckSize, handSize int) ([]int, error) { - if errs := ValidateParameters(deckSize, handSize); len(errs) > 0 { - return nil, errors.New(strings.Join(errs, ";")) - } - - return ShuffleAndDealToSlice(hashValue, deckSize, handSize), nil -} - -// ShuffleAndDealIntoHandWithValidation does validation and then ShuffleAndDealIntoHand -func ShuffleAndDealIntoHandWithValidation(hashValue uint64, deckSize int, hand []int) error { - if errs := ValidateParameters(deckSize, len(hand)); len(errs) > 0 { - return errors.New(strings.Join(errs, ";")) - } - ShuffleAndDealIntoHand(hashValue, deckSize, hand) - return nil +// DealIntoHand shuffles and deals according to the Dealer's parameters, +// using the given hashValue as the source of entropy and then +// returns the dealt cards as a slice of `int`. +// If `hand` has the correct length as Dealer's handSize, it will be used as-is and no allocations will be made. +// If `hand` is nil or too small, it will be extended (performing an allocation). +// If `hand` is too large, a sub-slice will be returned. +func (d *Dealer) DealIntoHand(hashValue uint64, hand []int) []int { + h := hand[:0] + d.Deal(hashValue, func(card int) { h = append(h, card) }) + return h } diff --git a/staging/src/k8s.io/apiserver/pkg/util/shufflesharding/shufflesharding_test.go b/staging/src/k8s.io/apiserver/pkg/util/shufflesharding/shufflesharding_test.go index 125c28da284..4e71a942a14 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/shufflesharding/shufflesharding_test.go +++ b/staging/src/k8s.io/apiserver/pkg/util/shufflesharding/shufflesharding_test.go @@ -17,55 +17,80 @@ limitations under the License. package shufflesharding import ( + "fmt" "math" "math/rand" + "reflect" "sort" - "strings" "testing" ) -func TestValidateParameters(t *testing.T) { +func TestRequiredEntropyBits(t *testing.T) { + tests := []struct { + name string + deckSize int + handSize int + expectedBits int + }{ + { + "deckSize: 1024 handSize: 6", + 1024, + 6, + 60, + }, + { + "deckSize: 512 handSize: 8", + 512, + 8, + 72, + }, + } + + for _, test := range tests { + bits := RequiredEntropyBits(test.deckSize, test.handSize) + if bits != test.expectedBits { + t.Errorf("test %s fails: expected %v but got %v", test.name, test.expectedBits, bits) + return + } + } +} + +func TestNewDealer(t *testing.T) { tests := []struct { name string deckSize int handSize int - errors []string + err error }{ { - "deckSize is < 0", + "deckSize <= 0", -100, 8, - []string{"deckSize is not positive"}, + fmt.Errorf("deckSize -100 or handSize 8 is not positive"), }, { - "handSize is < 0", - 128, - -100, - []string{"handSize is not positive"}, - }, - { - "deckSize is 0", + "handSize <= 0", + 100, 0, - 8, - []string{"deckSize is not positive"}, - }, - { - "handSize is 0", - 128, - 0, - []string{"handSize is not positive"}, + fmt.Errorf("deckSize 100 or handSize 0 is not positive"), }, { "handSize is greater than deckSize", - 128, - 129, - []string{"handSize is greater than deckSize"}, + 100, + 101, + fmt.Errorf("handSize 101 is greater than deckSize 100"), }, { - "deckSize: 128 handSize: 6", - 128, - 6, - nil, + "deckSize is impractically large", + 1 << 27, + 2, + fmt.Errorf("deckSize 134217728 is impractically large"), + }, + { + "required entropy bits is greater than MaxHashBits", + 512, + 8, + fmt.Errorf("required entropy bits of deckSize 512 and handSize 8 is greater than 60"), }, { "deckSize: 1024 handSize: 6", @@ -73,211 +98,92 @@ func TestValidateParameters(t *testing.T) { 6, nil, }, - { - "deckSize: 512 handSize: 8", - 512, - 8, - []string{"more than 60 bits of entropy required"}, - }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got := strings.Join(ValidateParameters(test.deckSize, test.handSize), ";") - expected := strings.Join(test.errors, ";") - if got != expected { - t.Errorf("test case %s got %q but expected %q", test.name, got, expected) + _, err := NewDealer(test.deckSize, test.handSize) + if !reflect.DeepEqual(err, test.err) { + t.Errorf("test %s fails: expected %v but got %v", test.name, test.err, err) return } }) } } -func BenchmarkValidateParameters(b *testing.B) { - deckSize, handSize := 512, 8 - for i := 0; i < b.N; i++ { - _ = ValidateParameters(deckSize, handSize) - } -} - -func TestShuffleAndDealWithValidation(t *testing.T) { +func TestCardDuplication(t *testing.T) { tests := []struct { - name string - deckSize int - handSize int - pick func(int) - validated bool + name string + deckSize int + handSize int }{ - { - "deckSize is < 0", - -100, - 8, - func(int) {}, - false, - }, - { - "handSize is < 0", - 128, - -100, - func(int) {}, - false, - }, - { - "deckSize is 0", - 0, - 8, - func(int) {}, - false, - }, - { - "handSize is 0", - 128, - 0, - func(int) {}, - false, - }, - { - "handSize is greater than deckSize", - 128, - 129, - func(int) {}, - false, - }, - { - "deckSize: 128 handSize: 6", - 128, - 6, - func(int) {}, - true, - }, - { - "deckSize: 1024 handSize: 6", - 1024, - 6, - func(int) {}, - true, - }, - { - "deckSize: 512 handSize: 8", - 512, - 8, - func(int) {}, - false, - }, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - if (ShuffleAndDealWithValidation(rand.Uint64(), test.deckSize, test.handSize, test.pick) == nil) != test.validated { - t.Errorf("test case %s fails", test.name) - return - } - }) - } -} - -func BenchmarkShuffleAndDeal(b *testing.B) { - hashValueBase := math.MaxUint64 / uint64(b.N) - deckSize, handSize := 512, 8 - pick := func(int) {} - for i := 0; i < b.N; i++ { - ShuffleAndDeal(hashValueBase*uint64(i), deckSize, handSize, pick) - } -} - -func TestShuffleAndDealToSliceWithValidation(t *testing.T) { - tests := []struct { - name string - deckSize int - handSize int - validated bool - }{ - { - "validation fails", - -100, - -100, - false, - }, { "deckSize = handSize = 4", 4, 4, - true, }, { "deckSize = handSize = 8", 8, 8, - true, }, { "deckSize = handSize = 10", 10, 10, - true, }, { "deckSize = handSize = 12", 12, 12, - true, }, { "deckSize = 128, handSize = 8", 128, 8, - true, }, { "deckSize = 256, handSize = 7", 256, 7, - true, }, { "deckSize = 512, handSize = 6", 512, 6, - true, }, } for _, test := range tests { hashValue := rand.Uint64() t.Run(test.name, func(t *testing.T) { - cards, err := ShuffleAndDealToSliceWithValidation(hashValue, test.deckSize, test.handSize) - if (err == nil) != test.validated { - t.Errorf("test case %s fails in validation check", test.name) + dealer, err := NewDealer(test.deckSize, test.handSize) + if err != nil { + t.Errorf("fail to create Dealer: %v", err) + return + } + hand := make([]int, 0) + hand = dealer.DealIntoHand(hashValue, hand) + + // check cards number + if len(hand) != int(test.handSize) { + t.Errorf("test case %s fails in cards number", test.name) return } - if test.validated { - // check cards number - if len(cards) != int(test.handSize) { - t.Errorf("test case %s fails in cards number", test.name) - return - } - - // check cards range and duplication - cardMap := make(map[int]struct{}) - for _, card := range cards { - if card < 0 || card >= int(test.deckSize) { - t.Errorf("test case %s fails in range check", test.name) - return - } - cardMap[card] = struct{}{} - } - if len(cardMap) != int(test.handSize) { - t.Errorf("test case %s fails in duplication check", test.name) + // check cards range and duplication + cardMap := make(map[int]struct{}) + for _, card := range hand { + if card < 0 || card >= int(test.deckSize) { + t.Errorf("test case %s fails in range check", test.name) return } + cardMap[card] = struct{}{} + } + if len(cardMap) != int(test.handSize) { + t.Errorf("test case %s fails in duplication check", test.name) + return } - }) - } -} -func BenchmarkShuffleAndDealToSlice(b *testing.B) { - hashValueBase := math.MaxUint64 / uint64(b.N) - deckSize, handSize := 512, 8 - for i := 0; i < b.N; i++ { - _ = ShuffleAndDealToSlice(hashValueBase*uint64(i), deckSize, handSize) + }) } } @@ -293,7 +199,7 @@ func ff(n, m int) int { } func TestUniformDistribution(t *testing.T) { - const spare = 64 - maxHashBits + const spare = 64 - MaxHashBits tests := []struct { deckSize, handSize int hashMax int @@ -304,6 +210,11 @@ func TestUniformDistribution(t *testing.T) { {70, 4, ff(70, 4)}, } for _, test := range tests { + dealer, err := NewDealer(test.deckSize, test.handSize) + if err != nil { + t.Errorf("fail to create Dealer: %v", err) + return + } handCoordinateMap := make(map[int]int) // maps coded hand to count of times seen fallingFactorial := ff(test.deckSize, test.handSize) @@ -314,7 +225,7 @@ func TestUniformDistribution(t *testing.T) { maxCount := permutations * int(math.Ceil(nff)) aHand := make([]int, test.handSize) for i := 0; i < test.hashMax; i++ { - ShuffleAndDealIntoHand(uint64(i), test.deckSize, aHand) + aHand = dealer.DealIntoHand(uint64(i), aHand) sort.IntSlice(aHand).Sort() handCoordinate := 0 for _, card := range aHand { @@ -345,3 +256,63 @@ func TestUniformDistribution(t *testing.T) { } return } + +func TestDealer_DealIntoHand(t *testing.T) { + dealer, _ := NewDealer(6, 6) + + tests := []struct { + name string + hand []int + expectedSize int + }{ + { + "nil slice", + nil, + 6, + }, + { + "empty slice", + make([]int, 0), + 6, + }, + { + "size: 6 cap: 6 slice", + make([]int, 6, 6), + 6, + }, + { + "size: 6 cap: 12 slice", + make([]int, 6, 12), + 6, + }, + { + "size: 4 cap: 4 slice", + make([]int, 4, 4), + 6, + }, + { + "size: 4 cap: 12 slice", + make([]int, 4, 12), + 6, + }, + { + "size: 10 cap: 10 slice", + make([]int, 10, 10), + 6, + }, + { + "size: 10 cap: 12 slice", + make([]int, 10, 12), + 6, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + h := dealer.DealIntoHand(0, test.hand) + if len(h) != test.expectedSize { + t.Errorf("test %s fails: expetced size %d but got %d", test.name, test.expectedSize, len(h)) + return + } + }) + } +}