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.

Sign up for GitHub

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

Jump to bottom

Save entry list compressed in extension state #416

Merged
merged 2 commits into from
Jan 16, 2025
Merged

Save entry list compressed in extension state #416

merged 2 commits into from
Jan 16, 2025

Conversation

MartinWeindel
Copy link
Member

How to categorize this PR?

/area control-plane
/area control-plane-migration
/kind enhancement

What this PR does / why we need it:
The DNSEntries of the shoot must be stored in the .status.state of the Extension resource for resurrection after control plane migration.
If a shoot has about 2000 entries, this state becomes so big, that the request size limit of etcd (1.5MB) is hit (etcdserver: request is too large).
To shift the maximum number of DNS entries which can be supported, the state is now compressed with the brotli algorithm. We expect to see compression ratios >= 6, so the limit should now be beyond 10,000 entries.

Which issue(s) this PR fixes:
Fixes #413

Special notes for your reviewer:

Release note:

The entry list stored in the extension resource status is now compressed to shift the limit before hitting the request size limit of etcd.

@MartinWeindel MartinWeindel requested review from a team as code owners January 15, 2025 08:55
@gardener-robot gardener-robot added needs/review Needs review area/control-plane Control plane related area/control-plane-migration Control plane migration related kind/enhancement Enhancement, improvement, extension size/l Size of pull request is large (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Jan 15, 2025
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 15, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 15, 2025
marc1404
marc1404 reviewed Jan 15, 2025
View reviewed changes
Copy link
Member

@marc1404 marc1404 left a comment

Choose a reason for hiding this comment

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

Great work!
I'm excited to have this extension state compression in so that the control plane migration problem would only occur in much larger workloads.

Makefile Show resolved Hide resolved
pkg/controller/common/state.go Outdated Show resolved Hide resolved
test/integration/lifecycle/state_test.go Show resolved Hide resolved
pkg/controller/common/compressedstate.go Outdated Show resolved Hide resolved
@gardener-robot gardener-robot added size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) and removed size/l Size of pull request is large (see gardener-robot robot/bots/size.py) labels Jan 15, 2025
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 15, 2025
marc1404
marc1404 approved these changes Jan 15, 2025
View reviewed changes
Copy link
Member

@marc1404 marc1404 left a comment

Choose a reason for hiding this comment

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

/lgtm

Thank you!

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review needs/second-opinion Needs second review by someone else labels Jan 15, 2025
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 15, 2025
@MartinWeindel MartinWeindel merged commit f2f0817 into master Jan 16, 2025
9 checks passed
@MartinWeindel MartinWeindel deleted the enh/compress-state branch January 16, 2025 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marc1404 marc1404 marc1404 approved these changes

Assignees
No one assigned
Labels
area/control-plane Control plane related area/control-plane-migration Control plane migration related kind/enhancement Enhancement, improvement, extension needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Extension state exceeds the default request size limit of etcd when cluster has > 2000 DNSEntries
6 participants
@MartinWeindel @marc1404 @gardener-robot-ci-1 @gardener-robot-ci-2 @gardener-robot-ci-3 @gardener-robot