Removal of getRandomlySelectedGS

When refactoring the allocator code for googleforgames#2143 I noticed that this function was never actually utilised for anything, so figured it made sense to remove it.
markmandel · Jun 21, 2021 · bfed18c · bfed18c
1 parent b115471
commit bfed18c
Show file tree

Hide file tree

Showing 2 changed files with 0 additions and 62 deletions.
diff --git a/pkg/gameserverallocations/allocator.go b/pkg/gameserverallocations/allocator.go
@@ -19,9 +19,7 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"fmt"
-	"math/rand"
 	"net/http"
-	"strconv"
 	"strings"
 	"time"
 
@@ -73,11 +71,6 @@ const (
 	secretClientKeyName  = "tls.key"
 	secretCACertName     = "ca.crt"
 	allocatorPort        = "443"
-
-	// Instead of selecting the top one, controller selects a random one
-	// from the topNGameServerCount of Ready gameservers
-	// to reduce the contention while allocating gameservers.
-	topNGameServerDefaultCount = 100
 )
 
 const (
@@ -109,7 +102,6 @@ type Allocator struct {
 	recorder                     record.EventRecorder
 	pendingRequests              chan request
 	readyGameServerCache         *ReadyGameServerCache
-	topNGameServerCount          int
 	remoteAllocationCallback     func(context.Context, string, grpc.DialOption, *pb.AllocationRequest) (*pb.AllocationResponse, error)
 	remoteAllocationTimeout      time.Duration
 	totalRemoteAllocationTimeout time.Duration
@@ -138,7 +130,6 @@ func NewAllocator(policyInformer multiclusterinformerv1.GameServerAllocationPoli
 		secretLister:                 secretInformer.Lister(),
 		secretSynced:                 secretInformer.Informer().HasSynced,
 		readyGameServerCache:         readyGameServerCache,
-		topNGameServerCount:          topNGameServerDefaultCount,
 		remoteAllocationTimeout:      remoteAllocationTimeout,
 		totalRemoteAllocationTimeout: totalRemoteAllocationTimeout,
 		remoteAllocationCallback: func(ctx context.Context, endpoint string, dialOpts grpc.DialOption, request *pb.AllocationRequest) (*pb.AllocationResponse, error) {
@@ -607,24 +598,6 @@ func Retry(backoff wait.Backoff, fn func() error) error {
 	return err
 }
 
-// getRandomlySelectedGS selects a GS from the set of Gameservers randomly. This will reduce the contentions
-func (c *Allocator) getRandomlySelectedGS(gsa *allocationv1.GameServerAllocation, bestGSList []agonesv1.GameServer) *agonesv1.GameServer {
-	seed, err := strconv.Atoi(gsa.ObjectMeta.ResourceVersion)
-	if err != nil {
-		seed = 1234567
-	}
-
-	ln := c.topNGameServerCount
-	if ln > len(bestGSList) {
-		ln = len(bestGSList)
-	}
-
-	startIndex := len(bestGSList) - ln
-	bestGSList = bestGSList[startIndex:]
-	index := rand.New(rand.NewSource(int64(seed))).Intn(ln)
-	return &bestGSList[index]
-}
-
 // newMetrics creates a new gsa latency recorder.
 func (c *Allocator) newMetrics(ctx context.Context) *metrics {
 	ctx, err := tag.New(ctx, latencyTags...)

diff --git a/pkg/gameserverallocations/controller_test.go b/pkg/gameserverallocations/controller_test.go
@@ -561,40 +561,6 @@ func TestControllerRunCacheSync(t *testing.T) {
 	assertCacheEntries(0)
 }
 
-func TestGetRandomlySelectedGS(t *testing.T) {
-	c, _ := newFakeController()
-	c.allocator.topNGameServerCount = 5
-	gsa := &allocationv1.GameServerAllocation{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: defaultNs,
-			Name:      "allocation",
-		},
-		Status: allocationv1.GameServerAllocationStatus{
-			State: allocationv1.GameServerAllocationUnAllocated,
-		},
-	}
-
-	_, gsList := defaultFixtures(10)
-
-	selectedGS := c.allocator.getRandomlySelectedGS(gsa, gsList)
-	assert.NotNil(t, "selectedGS can't be nil", selectedGS)
-	for i := 1; i <= 5; i++ {
-		expectedName := "gs" + strconv.Itoa(i)
-		assert.NotEqual(t, expectedName, selectedGS.ObjectMeta.Name)
-	}
-
-	_, gsList = defaultFixtures(5)
-
-	selectedGS = c.allocator.getRandomlySelectedGS(gsa, gsList)
-	assert.NotNil(t, "selectedGS can't be nil", selectedGS)
-
-	_, gsList = defaultFixtures(1)
-
-	selectedGS = c.allocator.getRandomlySelectedGS(gsa, gsList)
-	assert.NotNil(t, "selectedGS can't be nil", selectedGS)
-	assert.Equal(t, "gs1", selectedGS.ObjectMeta.Name)
-}
-
 func TestControllerAllocationUpdateWorkers(t *testing.T) {
 	t.Run("no error", func(t *testing.T) {
 		c, m := newFakeController()
@@ -1553,7 +1519,6 @@ func newFakeControllerWithTimeout(remoteAllocationTimeout time.Duration, totalRe
 	counter := gameservers.NewPerNodeCounter(m.KubeInformerFactory, m.AgonesInformerFactory)
 	api := apiserver.NewAPIServer(m.Mux)
 	c := NewController(api, healthcheck.NewHandler(), counter, m.KubeClient, m.KubeInformerFactory, m.AgonesClient, m.AgonesInformerFactory, remoteAllocationTimeout, totalRemoteAllocationTimeout)
-	c.allocator.topNGameServerCount = 1
 	c.recorder = m.FakeRecorder
 	c.allocator.recorder = m.FakeRecorder
 	return c, m