Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Support Processing Units #34

Merged
merged 11 commits into from
Sep 10, 2021
Merged

Support Processing Units #34

merged 11 commits into from
Sep 10, 2021

Conversation

apstndb
Copy link
Contributor

@apstndb apstndb commented Aug 10, 2021 •
edited
Loading

What this PR does / Why we need it

To keep compatibility, this PR simply adds

  • spec
    • maxProcessingUnits
    • minProcessingUnits
  • status
    • desiredProcessingUnits
    • currentProcessingUnits

and support both of *ProcessingUnits and *Nodes.

In wording, I want to propose:

  • Use "compute capacity" as the abstract term.
  • Use "processing units(PU)" as the base unit for compute capacity.
  • Use "nodes" as the supplementary unit which is equals to 1000 PU.

See also https://cloud.google.com/spanner/docs/compute-capacity?hl=en for the official use of these terms.

Which issue(s) this PR fixes

Fixes #26

@github-actions github-actions bot added size/L and removed size/XXL labels Aug 13, 2021
@apstndb apstndb marked this pull request as ready for review August 13, 2021 11:43
@apstndb apstndb requested a review from a team as a code owner August 13, 2021 11:43
rustycl0ck
rustycl0ck reviewed Aug 17, 2021
View reviewed changes
Copy link
Contributor

@rustycl0ck rustycl0ck left a comment

Choose a reason for hiding this comment

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

Just some queries to clarify my understanding

@apstndb @tkuchiki
Apply suggestions from code review
4d615a9 
Co-authored-by: tkuchiki <kuchiki.taku@gmail.com>
@apstndb apstndb requested a review from tkuchiki September 10, 2021 03:48
tkuchiki
tkuchiki approved these changes Sep 10, 2021
View reviewed changes
Copy link
Contributor

@tkuchiki tkuchiki left a comment

Choose a reason for hiding this comment

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

LGTM

@tkuchiki tkuchiki merged commit 766c015 into mercari:master Sep 10, 2021
@apstndb apstndb deleted the processing-units branch September 13, 2021 04:11
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@rustycl0ck rustycl0ck rustycl0ck left review comments

@tkuchiki tkuchiki tkuchiki approved these changes

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Suppor low cost instances(a.k.a. Processing Units)
3 participants
@apstndb @rustycl0ck @tkuchiki