balancer/randomsubsetting: Implementation of the random_subsetting LB policy

[marek-szews]

Implements [gRFC A68]https://github.com/grpc/proposal/blob/master/A68-random-subsetting.md

RELEASE NOTES:

• balancer/randomsubsetting: Implementation of the random_subsetting LB policy

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: easwars / name: Easwar Swaminathan (c4f8448, ddc8d09, f577218)
• ✅ login: marek-szews / name: Marek Szews (3c3f093, 7f43d9e, e8ff4a1)

[easwars]

@marek-szews : Could you please get the tests to pass. Thanks.

[easwars]

Is there a reason for this type to be exported. I don't see a reason here. So, if there is no reason, please unexport it. The fields can remain exported since you might want that for JSON marshal/unmarshal.

[marek-szews]

Yes, we need to use this type in the test function.

[easwars]

The test is also in the same package and should have access to unexported symbols.

[easwars]

Where is this default value specified?

[marek-szews]

In the previous proposal of config I have found somthing like this (PROP-423):
message RandomSubsettingLbConfig {
// subset_size indicates how many backends every client will be connected to.
// Default is 20.
google.protobuf.UInt32Value subset_size = 1;
But I saw in the most current version (PROTO-157) that this information has been removed. My suggestion is to set subsetSize to 2 by default, because 1 should suggest to use pick_first instead.

[easwars]

Please revert to using no defaults, both for the subset size and the child policy. Both need to be specified by the user, and if they are missing, the configuration needs to be rejected by returning an error from ParseConfig.

[easwars]

The gRFC only says the following:

    // The value is required and must be greater than 0.

Is the condition that the size be at least 2 specified somewhere else?

[marek-szews]

See the discussion at line 86

[easwars]

Line 86 of what?

The only thing we should be checking is whether the subset size is greater than 0 and if not, returning an error from this method.

[github-actions]

This PR is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

[dfawley]

Hi @marek-szews - I think we're waiting on you to make some updates so we can continue the review. We can discuss Friday but I wanted to ping this in part to prevent the stale bot from closing it.

[marek-szews]

Rework done, tests passed

[marek-szews]

In the previous proposal of config I have found somthing like this (PROP-423):
message RandomSubsettingLbConfig {
// subset_size indicates how many backends every client will be connected to.
// Default is 20.
google.protobuf.UInt32Value subset_size = 1;
But I saw in the most current version (PROTO-157) that this information has been removed. My suggestion is to set subsetSize to 2 by default, because 1 should suggest to use pick_first instead.

[easwars]

Rework done, tests passed

Tests are still failing. Please read this document for guidelines: https://github.com/grpc/grpc-go/blob/master/CONTRIBUTING.md

TL;dr

All tests need to be passing before your change can be merged. We recommend you run tests locally before creating your PR to catch breakages early on:

./scripts/vet.sh to catch vet errors.
go test -cpu 1,4 -timeout 7m ./... to run the tests.
go test -race -cpu 1,4 -timeout 7m ./... to run tests in race mode