Skip to content

Implement chunking for gzip encoder in deferredResponseWriter#130281

Merged
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
z1cheng:issue_130264
Feb 26, 2025
Merged

Implement chunking for gzip encoder in deferredResponseWriter#130281
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
z1cheng:issue_130264

Conversation

@z1cheng
Copy link
Copy Markdown
Member

@z1cheng z1cheng commented Feb 19, 2025 •
edited
Loading

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR is to implement chunking for gzip encoder in deferredResponseWriter, and add benchmark for gzip with/without chunking.

Which issue(s) this PR fixes:

Fixes #130264

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Feb 19, 2025
@k8s-ci-robot k8s-ci-robot added area/apiserver sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 19, 2025
z1cheng
z1cheng commented Feb 19, 2025
View reviewed changes
Comment thread staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go Outdated
serathius
serathius reviewed Feb 19, 2025
View reviewed changes
Comment thread staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go Outdated
serathius
serathius reviewed Feb 19, 2025
View reviewed changes
Comment thread staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers_test.go Outdated
liggitt
liggitt reviewed Feb 19, 2025
View reviewed changes
Comment thread staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers_test.go Outdated
Comment thread staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go Outdated
serathius
serathius reviewed Feb 20, 2025
View reviewed changes
Comment thread staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go Outdated
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 20, 2025
@cici37
Copy link
Copy Markdown
Contributor

cici37 commented Feb 20, 2025

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 20, 2025
serathius
serathius reviewed Feb 21, 2025
View reviewed changes
Comment thread staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers_test.go Outdated
@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Feb 22, 2025
@z1cheng z1cheng requested a review from liggitt February 24, 2025 14:18
fuweid
fuweid approved these changes Feb 24, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM(nb)

We can revisit buffer part after completing end-to-end evaluation with beta or rc version.

@serathius serathius mentioned this pull request Feb 24, 2025
Merged
serathius
serathius reviewed Feb 25, 2025
View reviewed changes
Comment thread staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go Outdated
@z1cheng
Copy link
Copy Markdown
Member Author

z1cheng commented Feb 25, 2025

/retest

liggitt
liggitt reviewed Feb 25, 2025
View reviewed changes
Comment thread staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go Outdated
liggitt
liggitt reviewed Feb 25, 2025
View reviewed changes
Comment thread staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go Outdated
@z1cheng z1cheng requested review from liggitt and serathius February 26, 2025 13:37
liggitt
liggitt reviewed Feb 26, 2025
View reviewed changes
Comment thread staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/writers.go Outdated
@liggitt
Copy link
Copy Markdown
Member

liggitt commented Feb 26, 2025

left comments about two final tweaks, then lgtm

@z1cheng
Implement chunking for gzip encoder in deferredResponseWriter
2472f49 
Signed-off-by: z1cheng <imchench@gmail.com>
@serathius
Copy link
Copy Markdown
Contributor

serathius commented Feb 26, 2025

/lgtm
Awesome work @z1cheng

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 26, 2025
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Feb 26, 2025

LGTM label has been added.

DetailsGit tree hash: d1da4b093e03047b5e98b211503226aecfb0196d

@liggitt
Copy link
Copy Markdown
Member

liggitt commented Feb 26, 2025

/lgtm
/approve

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Feb 26, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fuweid, liggitt, z1cheng

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 26, 2025
@z1cheng
Copy link
Copy Markdown
Member Author

z1cheng commented Feb 26, 2025

/retest

@k8s-ci-robot k8s-ci-robot merged commit 25dc6c9 into kubernetes:master Feb 26, 2025
@k8s-ci-robot k8s-ci-robot added this to the v1.33 milestone Feb 26, 2025
@z1cheng z1cheng deleted the issue_130264 branch February 27, 2025 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@liggitt liggitt liggitt left review comments

@wojtek-t wojtek-t wojtek-t left review comments

@hzxuzhonghu hzxuzhonghu Awaiting requested review from hzxuzhonghu

@serathius serathius Awaiting requested review from serathius

+1 more reviewer

@fuweid fuweid fuweid approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@liggitt liggitt

@serathius serathius

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Projects

None yet

Milestone

v1.33

Development

Successfully merging this pull request may close these issues.

Implement chunking for gzip encoder in deferredResponseWriter

7 participants

@z1cheng @cici37 @serathius @liggitt @k8s-ci-robot @wojtek-t @fuweid