-
Notifications
You must be signed in to change notification settings - Fork 86
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
Add option to random load balance to >1 server nodes #631
base: dev
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
|
@@ -697,6 +697,98 @@ func TestPeerSelectionRanking(t *testing.T) { | |
} | ||
} | ||
|
||
func TestZeroPeerConnectionCount(t *testing.T) { | ||
ch := testutils.NewClient(t, nil) | ||
defer ch.Close() | ||
err := ch.Peers().SetPeerConnectionCount(0) | ||
require.Error(t, err, "peerConnectionCount should not accept 0") | ||
} | ||
|
||
func TestPeerRandomSampling(t *testing.T) { | ||
const numIterations = 1000 | ||
|
||
testCases := []struct { | ||
numPeers int | ||
peerConnectionCount uint32 | ||
distMin float64 | ||
distMax float64 | ||
}{ | ||
// the higher `peerConnectionCount` is, the smoother the impact of uneven scores | ||
// become as we are random sampling among `peerConnectionCount` peers | ||
{numPeers: 10, peerConnectionCount: 1, distMin: 1000, distMax: 1000}, | ||
{numPeers: 10, peerConnectionCount: 2, distMin: 470, distMax: 530}, | ||
{numPeers: 10, peerConnectionCount: 5, distMin: 160, distMax: 240}, | ||
{numPeers: 10, peerConnectionCount: 10, distMin: 50, distMax: 150}, | ||
{numPeers: 10, peerConnectionCount: 15, distMin: 50, distMax: 150}, | ||
} | ||
|
||
for _, tc := range testCases { | ||
// Selected is a map from rank -> [peer, count] | ||
// It tracks how often a peer gets selected at a specific rank. | ||
selected := make([]map[string]int, tc.numPeers) | ||
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. why do you make a slice here, why can't we just create a singel map in the loop for 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. I am not sure if I follow. This is a similar test as 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. I am not sure if I follow. This is a similar test as |
||
for i := 0; i < tc.numPeers; i++ { | ||
selected[i] = make(map[string]int) | ||
} | ||
|
||
for i := 0; i < numIterations; i++ { | ||
ch := testutils.NewClient(t, nil) | ||
defer ch.Close() | ||
ch.SetRandomSeed(int64(i * 100)) | ||
// Using a strategy that has uneven scores | ||
strategy, _ := createScoreStrategy(0, 1) | ||
ch.Peers().SetStrategy(strategy) | ||
ch.Peers().SetPeerConnectionCount(tc.peerConnectionCount) | ||
|
||
for i := 0; i < tc.numPeers; i++ { | ||
hp := fmt.Sprintf("127.0.0.1:60%v", i) | ||
ch.Peers().Add(hp) | ||
} | ||
|
||
for i := 0; i < tc.numPeers; i++ { | ||
peer, err := ch.Peers().Get(nil) | ||
require.NoError(t, err, "Peers.Get failed") | ||
selected[i][peer.HostPort()]++ | ||
} | ||
} | ||
|
||
for _, m := range selected { | ||
testDistribution(t, m, tc.distMin, tc.distMax) | ||
} | ||
} | ||
|
||
} | ||
|
||
func BenchmarkGetPeerWithPeerConnectionCount1(b *testing.B) { | ||
doBenchmarkGetPeerWithPeerConnectionCount(b, 10, uint32(1)) | ||
} | ||
|
||
func BenchmarkGetPeerWithPeerConnectionCount10(b *testing.B) { | ||
doBenchmarkGetPeerWithPeerConnectionCount(b, 10, uint32(10)) | ||
} | ||
|
||
func doBenchmarkGetPeerWithPeerConnectionCount(b *testing.B, numPeers int, peerConnectionCount uint32) { | ||
ch := testutils.NewClient(b, nil) | ||
defer ch.Close() | ||
ch.SetRandomSeed(int64(100)) | ||
// Using a strategy that has uneven scores | ||
strategy, _ := createScoreStrategy(0, 1) | ||
ch.Peers().SetStrategy(strategy) | ||
ch.Peers().SetPeerConnectionCount(peerConnectionCount) | ||
|
||
for i := 0; i < numPeers; i++ { | ||
hp := fmt.Sprintf("127.0.0.1:60%v", i) | ||
ch.Peers().Add(hp) | ||
} | ||
b.ResetTimer() | ||
|
||
for i := 0; i < b.N; i++ { | ||
peer, _ := ch.Peers().Get(nil) | ||
if peer == nil { | ||
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. should this ever happen? If not, maybe we should do a 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. It shouldn't, I just added it to guard against compiler optimization not to artificially lower the runtime of the benchmark. Changed it to Fatal! 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. It shouldn't, I just added it to guard against compiler optimization not to artificially lower the runtime of the benchmark. Changed it to Fatal! |
||
b.Fatal("Just a dummy check to guard against compiler optimization") | ||
} | ||
} | ||
} | ||
|
||
func createScoreStrategy(initial, delta int64) (calc ScoreCalculator, retCount *atomic.Uint64) { | ||
var ( | ||
count atomic.Uint64 | ||
|
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.
can you add
peerConnectionCount: 2
, i imagine this will be a pretty small value normally?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.
Indeed as the impact of it get's smaller with every extra connection. Added the case for
2
!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.
Indeed as the impact of it get's smaller with every extra connection. Added the case for
2
!