Implements GetCounter and UpdateCounter on the SDK Server #3322

igooch · 2023-08-11T19:44:43Z

What type of PR is this?

/kind feature

What this PR does / Why we need it:

Implements GetCounter and UpdateCounter on the SDK Server for feature Counts and Lists.
Removes the LocalSDKServer code for UpdateCounter as this followed the previous version of the alpha.proto.

Which issue(s) this PR fixes:

Working on #2716

Special notes for your reviewer:

agones-bot · 2023-08-11T21:13:41Z

Build Failed 😱

Build Id: 42f7ef43-2752-41fd-b50f-c88a99a53b5d

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2023-08-11T22:46:36Z

Build Succeeded 👏

Build Id: ed0533ab-072b-48d1-ae7c-9a221c392268

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

https://5fac84a-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3322/head:pr_3322 && git checkout pr_3322
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.34.0-dev-5fac84a-amd64

agones-bot · 2023-08-19T00:19:24Z

Build Succeeded 👏

Build Id: 521463c0-afcf-4c92-a1ca-200096e28e9f

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

https://074353c-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3322/head:pr_3322 && git checkout pr_3322
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.35.0-dev-074353c-amd64

agones-bot · 2023-08-22T17:15:21Z

Build Failed 😱

Build Id: 456b779f-4fc0-4967-85ac-1f789239d1ee

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2023-08-22T17:24:31Z

Build Failed 😱

Build Id: 73ff5c59-740b-43b4-8272-c422c2e7c752

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

markmandel

Looks good! Dropped a bunch of comments mostly around how we can avoid some race conditions as part of the process of batching and keeping an in memory cache, and hopefully some help with some simplification.

pkg/sdkserver/sdkserver.go

agones-bot · 2023-08-24T16:41:12Z

Build Failed 😱

Build Id: 2a3ee945-f307-47b6-9248-1a5e4261c1ee

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2023-08-25T21:35:20Z

Build Failed 😱

Build Id: 8788b57b-a7d9-4916-a1d2-841bbd5d5af3

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2023-08-25T22:34:01Z

Build Failed 😱

Build Id: 444f2a60-415e-46a9-82d0-adad0a318946

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

pkg/sdkserver/sdkserver.go

agones-bot · 2023-08-26T20:02:30Z

Build Failed 😱

Build Id: 1c32d065-5350-4064-aaa1-fb5490152739

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2023-08-28T18:24:17Z

Build Failed 😱

Build Id: fb5db2c6-7c11-4137-bfa5-62de6c6a741a

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2023-08-28T22:34:26Z

Build Failed 😱

Build Id: bd7aaa9f-1748-4263-bb39-8c417e2c4719

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2023-08-29T16:49:56Z

Build Failed 😱

Build Id: ead5eefd-49eb-421d-bb14-86edc3c51181

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

pkg/sdkserver/sdkserver.go

pkg/sdkserver/sdkserver_test.go

google-oss-prow · 2023-09-09T00:20:12Z

New changes are detected. LGTM label has been removed.

agones-bot · 2023-09-09T00:54:43Z

Build Failed 😱

Build Id: 448b6b4a-b634-4989-ac4f-fc79216fff1f

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

markmandel · 2023-09-11T16:33:21Z

sigh

   gameserver_test.go:953: 
        	Error Trace:	/go/src/agones.dev/agones/test/e2e/gameserver_test.go:953
        	Error:      	Not equal: 
        	            	expected: "spec.template.spec.containers[0].resources.requests[cpu]"
        	            	actual  : "spec.template.spec.containers[0].resources.requests"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-spec.template.spec.containers[0].resources.requests[cpu]
        	            	+spec.template.spec.containers[0].resources.requests
        	Test:       	TestGameServerResourceValidation

If the GameServer has been successfully updated by the client in the updateCounter this updated version is stored in the SDK. If the s.gameServer() is called and the Generation of the gameServer is less than (older than) the version stored in the SDK, then the SDK version is returned.

The diff will be a single aggregated number that may be truncated if final Count of the Counter would be less than zero or greater than capacity. This means a request to increment or decrement a Counter may only be partially applied.

…ater than Capacity after decrementation Also adds tests.

This logic followed a previous version of the proto for UpdateCounter.

Largest change is to truncate any existing Count to the Capacity, so that Count is always less than or equal to Capacity on any change.

markmandel · 2023-09-11T17:07:13Z

Sorry - should have been clear - the sigh was because the flake is showing up. This PR is great 😄

Flaky: TestGameServerResourceValidation #3362

agones-bot · 2023-09-11T18:02:15Z

Build Failed 😱

Build Id: 5c115fa5-b443-4ce8-938f-5076991bf05a

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2023-09-11T21:19:05Z

Build Succeeded 👏

Build Id: a02ec040-866e-4706-a9be-fffc85696203

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

https://802f1cb-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3322/head:pr_3322 && git checkout pr_3322
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.35.0-dev-802f1cb-amd64

google-oss-prow bot added the do-not-merge/work-in-progress label Aug 11, 2023

google-oss-prow bot requested review from markmandel and roberthbailey August 11, 2023 19:44

google-oss-prow bot added the size/L label Aug 11, 2023

github-actions bot added the kind/feature New features for Agones label Aug 11, 2023

igooch force-pushed the arbitrary-counts-lists-2716 branch from 7bcc72d to afe8590 Compare August 11, 2023 21:20

igooch force-pushed the arbitrary-counts-lists-2716 branch from 5fac84a to 074353c Compare August 18, 2023 23:17

google-oss-prow bot added size/XL and removed size/L labels Aug 22, 2023

igooch force-pushed the arbitrary-counts-lists-2716 branch from 87fdb29 to c843318 Compare August 22, 2023 17:08

google-oss-prow bot added size/XXL and removed size/XL labels Aug 22, 2023

markmandel reviewed Aug 22, 2023

View reviewed changes

igooch force-pushed the arbitrary-counts-lists-2716 branch from a9169c9 to 4a45e93 Compare August 25, 2023 22:26

markmandel reviewed Aug 25, 2023

View reviewed changes

pkg/sdkserver/sdkserver.go Show resolved Hide resolved

igooch force-pushed the arbitrary-counts-lists-2716 branch from cd75892 to db7b6c4 Compare August 28, 2023 22:26

igooch commented Aug 29, 2023

View reviewed changes

pkg/sdkserver/sdkserver.go Outdated Show resolved Hide resolved

igooch commented Aug 29, 2023

View reviewed changes

pkg/sdkserver/sdkserver_test.go Outdated Show resolved Hide resolved

google-oss-prow bot added the approved label Sep 8, 2023

igooch force-pushed the arbitrary-counts-lists-2716 branch from e50af10 to 4178925 Compare September 9, 2023 00:20

google-oss-prow bot removed the lgtm label Sep 9, 2023

igooch added 17 commits September 11, 2023 16:49

Implements GetCounter and UpdateCounter on the SDK Server

cc16137

Adds UpdateCounter SDKServer test

c56aaa3

Changes Update Counter Request to batch and validate in the SDK

a3e159a

Cleans up duplicate code in sdkserver updateCounter

446dd3d

Removes the diffList for counterUpdateRequest

f6ebdcb

The diff will be a single aggregated number that may be truncated if final Count of the Counter would be less than zero or greater than capacity. This means a request to increment or decrement a Counter may only be partially applied.

Adds logic to allow for decrementing the Count even when Count is gre…

85fd807

…ater than Capacity after decrementation Also adds tests.

More tests for updateCounter

2403c75

Removes UpdateCounter from the LocalSDK

47ec032

This logic followed a previous version of the proto for UpdateCounter.

Adds in debug logs for when the UpdateCounter request truncates requests

2d8efe2

Generated Files

f6bc7f2

Removes file that should no longer be generated

163998e

Changes per review comments

919ec5e

Largest change is to truncate any existing Count to the Capacity, so that Count is always less than or equal to Capacity on any change.

Removing unused swagger model

a90279e

Generated files

4ffc4e2

Minor updates per review comments

2972e7e

Updates error messages to follow the Google Style Guide

53163c8

igooch force-pushed the arbitrary-counts-lists-2716 branch from 4178925 to 53163c8 Compare September 11, 2023 17:01

Adds debug log for flaky TestGameServerResourceValidation

802f1cb

markmandel merged commit 9deb15f into googleforgames:main Sep 11, 2023
3 checks passed

igooch mentioned this pull request Jan 18, 2024

Request for igooch to become Approver #3600

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implements GetCounter and UpdateCounter on the SDK Server #3322

Implements GetCounter and UpdateCounter on the SDK Server #3322

igooch commented Aug 11, 2023 •

edited

Loading

agones-bot commented Aug 11, 2023

agones-bot commented Aug 11, 2023

agones-bot commented Aug 19, 2023

agones-bot commented Aug 22, 2023

agones-bot commented Aug 22, 2023

markmandel left a comment

agones-bot commented Aug 24, 2023

agones-bot commented Aug 25, 2023

agones-bot commented Aug 25, 2023

agones-bot commented Aug 26, 2023

agones-bot commented Aug 28, 2023

agones-bot commented Aug 28, 2023

agones-bot commented Aug 29, 2023

google-oss-prow bot commented Sep 9, 2023

agones-bot commented Sep 9, 2023

markmandel commented Sep 11, 2023

markmandel commented Sep 11, 2023

agones-bot commented Sep 11, 2023

agones-bot commented Sep 11, 2023

Implements GetCounter and UpdateCounter on the SDK Server #3322

Implements GetCounter and UpdateCounter on the SDK Server #3322

Conversation

igooch commented Aug 11, 2023 • edited Loading

agones-bot commented Aug 11, 2023

agones-bot commented Aug 11, 2023

agones-bot commented Aug 19, 2023

agones-bot commented Aug 22, 2023

agones-bot commented Aug 22, 2023

markmandel left a comment

Choose a reason for hiding this comment

agones-bot commented Aug 24, 2023

agones-bot commented Aug 25, 2023

agones-bot commented Aug 25, 2023

agones-bot commented Aug 26, 2023

agones-bot commented Aug 28, 2023

agones-bot commented Aug 28, 2023

agones-bot commented Aug 29, 2023

google-oss-prow bot commented Sep 9, 2023

agones-bot commented Sep 9, 2023

markmandel commented Sep 11, 2023

markmandel commented Sep 11, 2023

agones-bot commented Sep 11, 2023

agones-bot commented Sep 11, 2023

igooch commented Aug 11, 2023 •

edited

Loading