Skip to content
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.

Sign up for GitHub

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

Jump to bottom

R4R: Refactor GetSelectionWithBias for addressbook #74

Merged
merged 1 commit into from
Jun 26, 2019
Merged

R4R: Refactor GetSelectionWithBias for addressbook #74

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 43 additions & 82 deletions p2p/pex/addrbook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"encoding/binary"
"fmt"
"math"
"math/rand"
"net"
"sync"
"time"
Expand Down Expand Up @@ -405,89 +406,11 @@ func (a *addrBook) GetSelectionWithBias(biasTowardsNewAddrs int) []*p2p.NetAddre
bookSize*getSelectionPercent/100)
numAddresses = cmn.MinInt(maxGetSelection, numAddresses)

selection := make([]*p2p.NetAddress, numAddresses)

oldBucketToAddrsMap := make(map[int]map[string]struct{})
var oldIndex int
newBucketToAddrsMap := make(map[int]map[string]struct{})
var newIndex int

// initialize counters used to count old and new added addresses.
// len(oldBucketToAddrsMap) cannot be used as multiple addresses can endup in the same bucket.
var oldAddressesAdded int
var newAddressesAdded int

// number of new addresses that, if possible, should be in the beginning of the selection
numRequiredNewAdd := percentageOfNum(biasTowardsNewAddrs, numAddresses)

selectionIndex := 0
ADDRS_LOOP:
for selectionIndex < numAddresses {
// biasedTowardsOldAddrs indicates if the selection can switch to old addresses
biasedTowardsOldAddrs := selectionIndex >= numRequiredNewAdd
// An old addresses is selected if:
// - the bias is for old and old addressees are still available or,
// - there are no new addresses or all new addresses have been selected.
// numAddresses <= a.nOld + a.nNew therefore it is guaranteed that there are enough
// addresses to fill the selection
pickFromOldBucket :=
(biasedTowardsOldAddrs && oldAddressesAdded < a.nOld) ||
a.nNew == 0 || newAddressesAdded >= a.nNew

bucket := make(map[string]*knownAddress)

// loop until we pick a random non-empty bucket
for len(bucket) == 0 {
if pickFromOldBucket {
oldIndex = a.rand.Intn(len(a.bucketsOld))
bucket = a.bucketsOld[oldIndex]
} else {
newIndex = a.rand.Intn(len(a.bucketsNew))
bucket = a.bucketsNew[newIndex]
}
}

// pick a random index
randIndex := a.rand.Intn(len(bucket))

// loop over the map to return that index
var selectedAddr *p2p.NetAddress
for _, ka := range bucket {
if randIndex == 0 {
selectedAddr = ka.Addr
break
}
randIndex--
}

// if we have selected the address before, restart the loop
// otherwise, record it and continue
if pickFromOldBucket {
if addrsMap, ok := oldBucketToAddrsMap[oldIndex]; ok {
if _, ok = addrsMap[selectedAddr.String()]; ok {
continue ADDRS_LOOP
}
} else {
oldBucketToAddrsMap[oldIndex] = make(map[string]struct{})
}
oldBucketToAddrsMap[oldIndex][selectedAddr.String()] = struct{}{}
oldAddressesAdded++
} else {
if addrsMap, ok := newBucketToAddrsMap[newIndex]; ok {
if _, ok = addrsMap[selectedAddr.String()]; ok {
continue ADDRS_LOOP
}
} else {
newBucketToAddrsMap[newIndex] = make(map[string]struct{})
}
newBucketToAddrsMap[newIndex][selectedAddr.String()] = struct{}{}
newAddressesAdded++
}

selection[selectionIndex] = selectedAddr
selectionIndex++
}

// if there are no enough old addrs, will choose new addr instead.
numRequiredNewAdd := cmn.MaxInt(percentageOfNum(biasTowardsNewAddrs, numAddresses), numAddresses-a.nOld)
selection := a.randomPickAddresses(bucketTypeNew, numRequiredNewAdd)
selection = append(selection, a.randomPickAddresses(bucketTypeOld, numAddresses-len(selection))...)
return selection
}

Expand Down Expand Up @@ -726,6 +649,44 @@ func (a *addrBook) addAddress(addr, src *p2p.NetAddress) error {
return nil
}

func (a *addrBook) randomPickAddresses(bucketType byte, num int) []*p2p.NetAddress {
var buckets []map[string]*knownAddress
switch bucketType {
case bucketTypeNew:
buckets = a.bucketsNew
case bucketTypeOld:
buckets = a.bucketsOld
default:
panic("unexpected bucketType")
}
total := 0
for _, bucket := range buckets {
total = total + len(bucket)
}
addresses := make([]*knownAddress, 0, total)
for _, bucket := range buckets {
for _, ka := range bucket {
addresses = append(addresses, ka)
}
}
selection := make([]*p2p.NetAddress, 0, num)
chosenSet := make(map[string]bool, num)
rand.Shuffle(total, func(i, j int) {
addresses[i], addresses[j] = addresses[j], addresses[i]
})
for _, addr := range addresses {
if chosenSet[addr.Addr.String()] {
continue
}
chosenSet[addr.Addr.String()] = true
selection = append(selection, addr.Addr)
if len(selection) >= num {
return selection
}
}
return selection
}

// Make space in the new buckets by expiring the really bad entries.
// If no bad entries are available we remove the oldest.
func (a *addrBook) expireNew(bucketIdx int) {
Expand Down
37 changes: 16 additions & 21 deletions p2p/pex/addrbook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"os"
"testing"

"github.com/stretchr/testify/require"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
cmn "github.com/tendermint/tendermint/libs/common"
"github.com/tendermint/tendermint/libs/log"
"github.com/tendermint/tendermint/p2p"
Expand Down Expand Up @@ -432,12 +432,12 @@ func TestPrivatePeers(t *testing.T) {

func testAddrBookAddressSelection(t *testing.T, bookSize int) {
// generate all combinations of old (m) and new addresses
for nOld := 0; nOld <= bookSize; nOld++ {
nNew := bookSize - nOld
dbgStr := fmt.Sprintf("book of size %d (new %d, old %d)", bookSize, nNew, nOld)
for nBookOld := 0; nBookOld <= bookSize; nBookOld++ {
nBookNew := bookSize - nBookOld
dbgStr := fmt.Sprintf("book of size %d (new %d, old %d)", bookSize, nBookNew, nBookOld)

// create book and get selection
book, fname := createAddrBookWithMOldAndNNewAddrs(t, nOld, nNew)
book, fname := createAddrBookWithMOldAndNNewAddrs(t, nBookOld, nBookNew)
defer deleteTempFile(fname)
addrs := book.GetSelectionWithBias(biasToSelectNewPeers)
assert.NotNil(t, addrs, "%s - expected a non-nil selection", dbgStr)
Expand All @@ -457,27 +457,25 @@ func testAddrBookAddressSelection(t *testing.T, bookSize int) {
// Given:
// n - num new addrs, m - num old addrs
// k - num new addrs expected in the beginning (based on bias %)
// i=min(n, k), aka expFirstNew
// i=min(n, max(k,r-m)), aka expNew
// j=min(m, r-i), aka expOld
//
// We expect this layout:
// indices: 0...i-1 i...i+j-1 i+j...r
// addresses: N0..Ni-1 O0..Oj-1 Ni...
// indices: 0...i-1 i...i+j-1
// addresses: N0..Ni-1 O0..Oj-1
//
// There is at least one partition and at most three.
var (
k = percentageOfNum(biasToSelectNewPeers, nAddrs)
expFirstNew = cmn.MinInt(nNew, k)
expOld = cmn.MinInt(nOld, nAddrs-expFirstNew)
expNew = nAddrs - expOld
expLastNew = expNew - expFirstNew
k = percentageOfNum(biasToSelectNewPeers, nAddrs)
expNew = cmn.MinInt(nNew, cmn.MaxInt(k, nAddrs-nBookOld))
expOld = cmn.MinInt(nOld, nAddrs-expNew)
)

// Verify that the number of old and new addresses are as expected
if nNew < expNew || nNew > expNew {
if nNew != expNew {
t.Fatalf("%s - expected new addrs %d, got %d", dbgStr, expNew, nNew)
}
if nOld < expOld || nOld > expOld {
if nOld != expOld {
t.Fatalf("%s - expected old addrs %d, got %d", dbgStr, expOld, nOld)
}

Expand All @@ -496,15 +494,12 @@ func testAddrBookAddressSelection(t *testing.T, bookSize int) {
case expOld == 0: // all new addresses
expSeqLens = []int{nAddrs}
expSeqTypes = []int{1}
case expFirstNew == 0: // all old addresses
case expNew == 0: // all old addresses
expSeqLens = []int{nAddrs}
expSeqTypes = []int{2}
case nAddrs-expFirstNew-expOld == 0: // new addresses, old addresses
expSeqLens = []int{expFirstNew, expOld}
case nAddrs-expNew-expOld == 0: // new addresses, old addresses
expSeqLens = []int{expNew, expOld}
expSeqTypes = []int{1, 2}
default: // new addresses, old addresses, new addresses
expSeqLens = []int{expFirstNew, expOld, expLastNew}
expSeqTypes = []int{1, 2, 1}
}

assert.Equal(t, expSeqLens, seqLens,
Expand Down