Make batchWaitTime configurable in the Allocator

[valentintorikian]

Is your feature request related to a problem? Please describe.
We'd like to make the batchWaitTime configurable on the Alocator. On certain allocation pattern, the hard-coded 500ms sleep is too aggressive and degrading allocation latency.

Describe the solution you'd like
Make the batchWaitTime configurable through an environment variable, with a default set to 500ms to avoid any breaking change.

Describe alternatives you've considered

• Change the hardcoded value batchWaitTime instead of making it configurable.
• Getting rid of the sleep altogether.

Those two solutions would lead to a change in existing behavior so I think they are not to be considered.

Additional context
From our internal testing the impact on CPU usage is negligible but the uplift in p95 allocation latency as well as the throughput in allocation per second is significant.
In any case the default value won't change so existing behaviour is kept intact.

[roberthbailey]

Relevant code block:

agones/pkg/gameserverallocations/allocator.go

Lines 457 to 537 in a8993fe

	// ListenAndAllocate is a blocking function that runs in a loop
	// looking at c.requestBatches for batches of requests that are coming through.
	func (c *Allocator) ListenAndAllocate(ctx context.Context, updateWorkerCount int) {
	// setup workers for allocation updates. Push response values into
	// this queue for concurrent updating of GameServers to Allocated
	updateQueue := c.allocationUpdateWorkers(ctx, updateWorkerCount)
	// Batch processing strategy:
	// We constantly loop around the below for loop. If nothing is found in c.pendingRequests, we move to
	// default: which will wait for half a second, to allow for some requests to backup in c.pendingRequests,
	// providing us with a batch of Allocation requests in that channel
	// Once we have 1 or more requests in c.pendingRequests (which is buffered to 100), we can start the batch process.
	// Assuming this is the first run (either entirely, or for a while), list will be nil, and therefore the first
	// thing that will be done is retrieving the Ready GameServers and sorting them for this batch via
	// c.listSortedReadyGameServers(). This list is maintained as we flow through the batch.
	// We then use findGameServerForAllocation to loop around the sorted list of Ready GameServers to look for matches
	// against the preferred and required selectors of the GameServerAllocation. If there is an error, we immediately
	// pass that straight back to the response channel for this GameServerAllocation.
	// Assuming we find a matching GameServer to our GameServerAllocation, we remove it from the list and the backing
	// Ready GameServer cache.
	// We then pass the found GameServers into the updateQueue, where there are updateWorkerCount number of goroutines
	// waiting to concurrently attempt to move the GameServer into an Allocated state, and return the result to
	// GameServerAllocation request's response channel
	// Then we get the next item off the batch (c.pendingRequests), and do this all over again, but this time, we have
	// an already sorted list of GameServers, so we only need to find one that matches our GameServerAllocation
	// selectors, and put it into updateQueue
	// The tracking of requestCount >= maxBatchBeforeRefresh is necessary, because without it, at high enough load
	// the list of GameServers that we are using to allocate would never get refreshed (list = nil) with an updated
	// list of Ready GameServers, and you would eventually never be able to Allocate anything as long as the load
	// continued.
	var list []*agonesv1.GameServer
	requestCount := 0
	for {
	select {
	case req := <-c.pendingRequests:
	// refresh the list after every 100 allocations made in a single batch
	requestCount++
	if requestCount >= maxBatchBeforeRefresh {
	list = nil
	requestCount = 0
	}
	if list == nil {
	list = c.allocationCache.ListSortedGameServers()
	}
	gs, index, err := findGameServerForAllocation(req.gsa, list)
	if err != nil {
	req.response <- response{request: req, gs: nil, err: err}
	continue
	}
	// remove the game server that has been allocated
	list = append(list[:index], list[index+1:]...)
	if err := c.allocationCache.RemoveGameServer(gs); err != nil {
	// this seems unlikely, but lets handle it just in case
	req.response <- response{request: req, gs: nil, err: err}
	continue
	}
	updateQueue <- response{request: req, gs: gs.DeepCopy(), err: nil}
	case <-ctx.Done():
	return
	default:
	list = nil
	requestCount = 0
	// slow down cpu churn, and allow items to batch
	time.Sleep(batchWaitTime)
	}
	}
	}

[roberthbailey]

You might run into any other issues decreasing the batching time, e.g. more load on system listing and sorting gameservers. But if the batch time is configurable then you can tune the balance for your game between the allocation latency and the amount of CPU you are willing to churn.

[markmandel]

From our internal testing the impact on CPU usage is negligible but the uplift in p95 allocation latency as well as the throughput in allocation per second is significant.

Curious - what change did you make to the batch time that made this change?

[valentintorikian]

We built two images with different hardcoded batchWaitTime, one configured with 200ms and one with 100ms. And ran an allocation scenario consisting of around 0.5alloc/second.

The node hosting the controller is an ec2 t3.xlarge

With the default 500ms value we have the following results:
p99 = ~600ms
p95 = ~250ms
p90 = ~240ms
p50 = ~175ms

With the 200ms value:
p99 = ~235ms
p95 = ~185ms
p90 = ~115ms
p50 = ~65ms

With the 100ms value:
p99 = ~245ms
p95 = ~137ms
p90 = ~100ms
p50 = ~65ms

CPU/memory usage was comparable accross all three test cases.

[markmandel]

How did you see it changing throughput? What sort of scenario did you run it against? (i.e. size of cluster, churn in cluster, etc?)

[valentintorikian]

Cluster is configured with a fleet of 200 GS, they are shutdown 60s after allocation.
The cluster is composed of 8 nodes:

• 6 nodes (m5.2xlarge) dedicated to GS,
• 2 nodes (t3.xlarge) for support workloads

Coming back to our monitoring data:

The annotation are as follow 500ms test, 200ms test, 100ms test.

This is with a 5m rate interval

The allocation rate uplift is minor but present, although our goal is to reduce the latency as much as possible, regardless of CPU churn.

[markmandel]

I guess my thoughts here are:

1. Is 200 GameServers within a cluster of 8 nodes in a cluster a usual amount for your workload?
2. Is the churn rate in the cluster that you currently have indicative of what you will have in production
3. Once the churn rate increases, will the bottleneck of etcd basically remove any performance gains you are likely to get from making this change.
4. Is this an optimisation that works in isolation for this specific test case, but when facing a real world test is not actually going to change much? (As I'm sure you are aware, Kubernetes performance metrics change drastically based on cluster size, pod churn rate, etc).

Not to say I'm against making this a configurable value - just wanting to make sure that the effort to change it is worth it, and we aren't looking at a premature optimisation.

[valentintorikian]

GameServer count per node as well as cluster topology (node size AND node count) can vary greatly based on which game is consuming Agones.

Churn rate is indeed artificially high in those test cases indeed, and the tests are very synthetic in the rate of allocation.

With all that taken into account, making that sleep configurable would allow us to better tailor the performance on a game per game basis.

I understand your concern of premature optimization, but from my point of view, the effort to get that additional knob is pretty minimal as well as being backward compatible because we retain the default 500ms value.

[markmandel]

Totally fair enough - just wanted to ask the question to make sure 👍🏻