Fix for race condition: Allocation of Deleting GameServers Possible #367
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If an allocation occurred during a Fleet scale down, or during a update of a Fleet, it was entirely possible for those parallel delete operations to be applied to a GameServer that was being allocated at the same time. This is mainly because the client-go informer cache is lazily consistent, but also because there was nothing preventing a `Delete()` of a `GameServer` from happening while allocating a specific `GameServer`. To solve this, there are two strategies implemented here: 1. Share the `allocationLock` `sync.Mutex` between controllers such that allocations cannot occur while `GameServer` Deletes for Fleet resizing/update are happening and vice versa. 2. use `cache.WaitForCacheSync` to bring the cluster informer up to date, to remove the chance for non-updated information about `GameServers` to be acted upon. The shared lock is a quite broad approach - down the line, we could refine this to being per `Fleet`, or per `GameServerSet`, if we find this to be a bottleneck, but the priority here was to get something working that resolves the issue, and we can optimise as needed from here. There are also e2e tests specifically designed for catching these race conditions as well.
markmandel
added
kind/bug
|
Build Succeeded 👏 Build Id: 0c0d9f54-7cb9-471d-aee0-4df518bfc905 The following development artifacts have been built, and will exist for the next 30 days:
(experimental) To install this version:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If an allocation occurred during a Fleet scale down, or during a update of a Fleet, it was entirely possible for those parallel delete operations to be applied to a GameServer that was being allocated at the same time.
This is mainly because the client-go informer cache is lazily consistent, but also because there was nothing preventing a
Delete()of aGameServerfrom happening while allocating a specificGameServer.To solve this, there are two strategies implemented here:
allocationLocksync.Mutexbetween controllers such that allocations cannot occur whileGameServerDeletes for Fleet resizing/update are happening and vice versa.cache.WaitForCacheSyncto bring the cluster informer up to date, to remove the chance for non-updated information aboutGameServersto beacted upon.
The shared lock is a quite broad approach - down the line, we could refine this to being per
Fleet, or perGameServerSet, if we find this to be a bottleneck, but the priority here was to get something working that resolvesthe issue, and we can optimise as needed from here.There are also e2e tests specifically designed for catching these race conditions as well.