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

admission: enable admission control #68535

Merged
merged 1 commit into from
Oct 28, 2021

Conversation

sumeerbhola
Copy link
Collaborator

@sumeerbhola sumeerbhola commented Aug 6, 2021

Also updated existing roachtests that explicitly enabled
admission control to now disable admission control, so
that we can continue to compare performance of the two
settings.

Release note (ops change): The cluster settings affecting
the admission control system enablement are now set to
defaults that enable admission control.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@sumeerbhola
Copy link
Collaborator Author

this is just to get a full run of unit tests and roachtests on CI

@jordanlewis
Copy link
Member

If this passes basic tests, LGTM to merge and then revert Monday. Add annotations to roachperf like this: https://github.com/cockroachdb/roachperf/commit/7abd2af2bf5109376581c299c1a4a47f11354ee3

@RaduBerinde
Copy link
Member

We should merge this soon. Along with it, won't we need to update the roachtests? The AdmissionControlEnabled flag would become a no-op, seems like we'd want to invert it and have some "noadmission" variants for a few tests.

Copy link
Collaborator Author

@sumeerbhola sumeerbhola left a comment

Choose a reason for hiding this comment

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

Along with it, won't we need to update the roachtests? The AdmissionControlEnabled flag would become a no-op, seems like we'd want to invert it and have some "noadmission" variants for a few tests.

Done

TFTR!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis and @RaduBerinde)

@sumeerbhola sumeerbhola changed the title [DNM] admission: enable admission control admission: enable admission control Oct 27, 2021
Copy link
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis)

Also updated existing roachtests that explicitly enabled
admission control to now disable admission control, so
that we can continue to compare performance of the two
settings.

Release note (ops change): The cluster settings affecting
the admission control system enablement are now set to
defaults that enable admission control.
@sumeerbhola
Copy link
Collaborator Author

bors r+

@craig
Copy link
Contributor

craig bot commented Oct 28, 2021

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Oct 28, 2021

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants