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.

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

Docs: Counters with High Density #4085

Merged
merged 2 commits into from
Jan 13, 2025

Conversation

markmandel
Copy link
Collaborator

What type of PR is this?

Uncomment only one /kind <> line, press enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking
/kind bug
/kind cleanup

/kind documentation

/kind feature
/kind hotfix
/kind release

What this PR does / Why we need it:

Implements documentation for the High Density Integration pattern using Counters.

Leaves in the "label locking" technique as well, as it is still a valid technique!

Which issue(s) this PR fixes:

Closes #3655

Special notes for your reviewer:

@markmandel markmandel added the kind/feature New features for Agones label Jan 8, 2025
@markmandel markmandel requested a review from igooch January 8, 2025 00:24
@github-actions github-actions bot added kind/documentation Documentation for Agones size/M labels Jan 8, 2025
@agones-bot
Copy link
Collaborator

Build Succeeded 🥳

Build Id: f85b42a8-0f9b-4f86-a076-a494a7b54664

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):

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/4085/head:pr_4085 && git checkout pr_4085
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.47.0-dev-967f7a9

Copy link
Collaborator

@igooch igooch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits, otherwise LGTM

are available on a given a `GameServer` from the list of potentially Ready or Allocated `GameServers` when making an
allocation request.

By also using Counters, we can provide Agones the allocation metadata it needs to pack appropriately across the high
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a link somewhere here to your note on consistency below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure it's necessary. It is on the page below. 🤔

@agones-bot
Copy link
Collaborator

Build Succeeded 🥳

Build Id: 1b600f63-2b52-4892-a4be-15567d3c837e

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):

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/4085/head:pr_4085 && git checkout pr_4085
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.47.0-dev-1c27dbb

Implements documentation for the High Density Integration pattern
using Counters.

Leaves in the "label locking" technique as well, as it is still a valid
technique!

Closes googleforgames#3655
apiVersion: allocation.agones.dev/v1
kind: GameServerAllocation
spec:
scheduling: Packed
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A side note that when scheduling is packed the priorities are a tie-breaker after all our other logic around node utilization etc., and the distributed uses priorities as the sorting strategy that's probably worth mentioning somewhere in this doc?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh that's a good point! Lemme put something in there. Also remembering we have #3652 as well to update those docs.

I've softened the language, since it's not a hard and fast rule, like you said, also put a Info box at the end.

priorities:
- type: Counter
key: rooms
order: Ascending # Ensures the least full "rooms" get allocated first.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
order: Ascending # Ensures the least full "rooms" get allocated first.
order: Ascending # Ensures the "rooms" with least available capacity (most full rooms) get allocated first.

Or something to that effect might make this clearer.

Copy link
Collaborator Author

@markmandel markmandel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good callouts. Updated! Lemme know what you think!

apiVersion: allocation.agones.dev/v1
kind: GameServerAllocation
spec:
scheduling: Packed
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh that's a good point! Lemme put something in there. Also remembering we have #3652 as well to update those docs.

I've softened the language, since it's not a hard and fast rule, like you said, also put a Info box at the end.

are available on a given a `GameServer` from the list of potentially Ready or Allocated `GameServers` when making an
allocation request.

By also using Counters, we can provide Agones the allocation metadata it needs to pack appropriately across the high
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure it's necessary. It is on the page below. 🤔

@markmandel markmandel force-pushed the docs/high-density-counters branch from 1c27dbb to 2c70de8 Compare January 10, 2025 20:56
@agones-bot
Copy link
Collaborator

Build Succeeded 🥳

Build Id: f3c11d1d-fcf0-4d80-a863-e3878a840156

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):

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/4085/head:pr_4085 && git checkout pr_4085
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.47.0-dev-2c70de8

@markmandel
Copy link
Collaborator Author

Just a heads up, I don't have the power to merge anymore 😆 (I should probably apply for approval access? maybe?)

@igooch igooch merged commit aa4032d into googleforgames:main Jan 13, 2025
4 checks passed
@igooch
Copy link
Collaborator

igooch commented Jan 13, 2025

Just a heads up, I don't have the power to merge anymore 😆 (I should probably apply for approval access? maybe?)

Pretty sure you've earned it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/documentation Documentation for Agones kind/feature New features for Agones size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs | Integration Patterns: Update High Density GameServers to use a Counter
3 participants