-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
roachpb: InternalServer API for tenant settings #74268
Conversation
Friendly ping. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
High level LGTM
Reviewed 1 of 3 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt and @RaduBerinde)
pkg/roachpb/api.proto, line 2631 at r1 (raw file):
// server side (e.g. to make it maintain very little state per tenant). message TenantSettingsEvent { // Tier indicates the tier and is either 1 or 2.
Any interest in a gogoproto.casttype
?
Code quote:
// Tier indicates the tier and is either 1 or 2.
uint32 tier = 1;
pkg/roachpb/api.proto, line 2646 at r1 (raw file):
// Overrides contains: // - all current setting overrides for the given tier if Incremental is // false; or
nit: trailing spaces
09705e6
to
36d2fdc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TFTR!
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner and @dt)
pkg/roachpb/api.proto, line 2631 at r1 (raw file):
Previously, ajwerner wrote…
Any interest in a
gogoproto.casttype
?
Good idea, done. While writing this I also thought about whether precedence
would be more descriptive than tier
.
36d2fdc
to
906d783
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like "precedence" too
906d783
to
63fd95f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like "precedence" too
Done! TFTR!
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner)
pkg/roachpb/api.proto, line 2651 at r5 (raw file):
// fields). repeated TenantSetting overrides = 3;
Moved this error field, I must have put it inside TenantSetting
by accident.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner and @RaduBerinde)
pkg/roachpb/api.proto, line 2631 at r1 (raw file):
Previously, RaduBerinde wrote…
Good idea, done. While writing this I also thought about whether
precedence
would be more descriptive thantier
.
Also +1 on precedence
This commit introduces the API that the tenants will use to obtain and list for updates to tenant setting overrides. The API was designed to allow for maximum flexibility on the server side so that it can be implemented as a range feed without any extra state (if necessary). Release note: None
63fd95f
to
024cb0e
Compare
bors r+ |
Build succeeded: |
This commit introduces the API that the tenants will use to obtain and
list for updates to tenant setting overrides. The API was designed to
allow for maximum flexibility on the server side so that it can be
implemented as a range feed without any extra state (if necessary).
Release note: None