Skip to content

Implement streaming proto list encoder#129407

Merged
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
serathius:streaming-proto-list-encoder
Mar 12, 2025
Merged

Implement streaming proto list encoder#129407
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
serathius:streaming-proto-list-encoder

Conversation

@serathius
Copy link
Copy Markdown
Contributor

@serathius serathius commented Dec 26, 2024

Mirrors #129334 for Protobuf

/kind feature

Implements proposal from #129304 where streaming protobuf respones reduced the memory used by apiserver 3-6x times. Validated on configmaps and pods objects with 5 concurrent list each returning 1.5GB of data.

Improved how the API server responds to **list** requests where the response format negotiates to Protobuf. List responses in Protobuf are marshalled one element at the time, drastically reducing memory needed to serve large collections. Streaming list responses can be disabled via the `StreamingCollectionEncodingToProtobuf` feature gate.

/cc @mborsz @wojtek-t @jpbetz @deads2k @p0lyn0mial
/sig api-machinery

@k8s-ci-robot k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Dec 26, 2024
@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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. area/apiserver labels Dec 26, 2024
@serathius serathius mentioned this pull request Dec 26, 2024
Closed
@serathius serathius force-pushed the streaming-proto-list-encoder branch from e09e329 to 3fe21b5 Compare December 30, 2024 10:53
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. area/code-generation area/dependency Issues or PRs related to dependency changes area/kube-proxy area/kubeadm area/kubectl area/provider/gcp Issues or PRs related to gcp provider area/test kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cli Categorizes an issue or PR as relevant to SIG CLI. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 30, 2024
@serathius
Copy link
Copy Markdown
Contributor Author

serathius commented Mar 4, 2025

/retest

@serathius serathius force-pushed the streaming-proto-list-encoder branch from e16590b to bfcb7af Compare March 5, 2025 18:00
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 5, 2025
@serathius serathius force-pushed the streaming-proto-list-encoder branch from bfcb7af to 68d8b6e Compare March 5, 2025 18:00
@serathius
Copy link
Copy Markdown
Contributor Author

serathius commented Mar 5, 2025

/retest

@serathius
Copy link
Copy Markdown
Contributor Author

serathius commented Mar 6, 2025

ping @liggitt

@serathius serathius changed the title Streaming proto list encoder Implement streaming proto list encoder Mar 6, 2025
@serathius serathius mentioned this pull request Mar 6, 2025
Closed
14 tasks
fuweid
fuweid approved these changes Mar 6, 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)!

@serathius
Copy link
Copy Markdown
Contributor Author

serathius commented Mar 10, 2025

ping @liggitt

liggitt
liggitt reviewed Mar 10, 2025
View reviewed changes
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections_test.go Outdated
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/protobuf.go Outdated
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections.go
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections.go
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections.go Outdated
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections.go Outdated
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections.go Outdated
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections.go Outdated
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections.go Outdated
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections.go Outdated
@serathius serathius force-pushed the streaming-proto-list-encoder branch 3 times, most recently from 85db384 to 7b39d57 Compare March 10, 2025 22:09
@serathius
Copy link
Copy Markdown
Contributor Author

serathius commented Mar 10, 2025

Addressed the commends, hopefully all of them as I worked on 2 different machines and didn't transfer it fully.

serathius
serathius commented Mar 10, 2025
View reviewed changes
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/types_proto.go
size := 0

// reuse the buffer for varint marshaling
varintBuffer := make([]byte, maxUint64VarIntLength)
Copy link
Copy Markdown
Contributor Author

@serathius serathius Mar 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Taken from PoC, but not a huge fan of local optimization like this. Maybe we could use allocator for that? But would rather do it in followup PR.

liggitt
liggitt reviewed Mar 11, 2025
View reviewed changes
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/types_proto.go Outdated
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections.go Outdated
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/protobuf.go
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/protobuf.go
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections.go
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections.go Outdated
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections.go
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections.go
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections.go Outdated
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/protobuf.go Outdated
@serathius
Copy link
Copy Markdown
Contributor Author

serathius commented Mar 11, 2025

Done, I think.

liggitt
liggitt reviewed Mar 11, 2025
View reviewed changes
Comment thread staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/collections.go Outdated
@liggitt
Copy link
Copy Markdown
Member

liggitt commented Mar 11, 2025

/lgtm
/approve

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Mar 11, 2025

LGTM label has been added.

DetailsGit tree hash: e94f5652c07460dc2201a138e3f071341fa37bd2

@enj enj mentioned this pull request Mar 18, 2025
Closed
@xrmzju xrmzju mentioned this pull request Sep 9, 2025
Open
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Oct 5, 2025

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@github-actions github-actions Bot mentioned this pull request Oct 16, 2025
Merged
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

@deads2k deads2k Awaiting requested review from deads2k

@jpbetz jpbetz Awaiting requested review from jpbetz

@mborsz mborsz Awaiting requested review from mborsz

@p0lyn0mial p0lyn0mial Awaiting requested review from p0lyn0mial

@wojtek-t wojtek-t Awaiting requested review from wojtek-t

+4 more reviewers

@neolit123 neolit123 neolit123 left review comments

@haosdent haosdent haosdent left review comments

@fuweid fuweid fuweid approved these changes

@hashim21223445 hashim21223445 hashim21223445 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@liggitt liggitt

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver area/code-generation area/dependency Issues or PRs related to dependency changes area/kube-proxy area/kubectl area/provider/gcp Issues or PRs related to gcp provider area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API 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 Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/release Categorizes an issue or PR as relevant to SIG Release. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on. wg/device-management Categorizes an issue or PR as relevant to WG Device Management.

Projects

API Reviews
Status: API review completed, 1.33
Dynamic Resource Allocation
Archived in project
SIG Apps
Archived in project
SIG Auth
Archived in project
SIG CLI
Archived in project
SIG Node CI/Test Board
Archived in project

Milestone

v1.33

Development

Successfully merging this pull request may close these issues.

10 participants

@serathius @k8s-triage-robot @fedebongio @liggitt @k8s-ci-robot @neolit123 @haosdent @fuweid @hashim21223445 @benjaminapetersen