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

k/h/metadata: use fragmented vec for topic metadata #15751

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

rockwotj
Copy link
Contributor

For workloads with a large amount of topics, this topics vector can grow
to be an oversized allocation.

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.3.x
  • v23.2.x
  • v23.1.x

Release Notes

Improvements

  • Prevent metadata oversized allocations when requesting many topics.

oleiman
oleiman previously approved these changes Dec 19, 2023
Copy link
Member

@oleiman oleiman left a comment

Choose a reason for hiding this comment

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

lgtm. couple extremely not blocking nits

src/v/kafka/server/handlers/metadata.cc Outdated Show resolved Hide resolved
src/v/kafka/server/handlers/metadata.cc Outdated Show resolved Hide resolved
@rockwotj
Copy link
Contributor Author

Force push: remove out of date comment

oleiman
oleiman previously approved these changes Dec 19, 2023
@rockwotj
Copy link
Contributor Author

Force push: use std::ranges::move

oleiman
oleiman previously approved these changes Dec 19, 2023
BenPope
BenPope previously approved these changes Dec 19, 2023
Copy link
Member

@BenPope BenPope left a comment

Choose a reason for hiding this comment

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

LGTM.

src/v/kafka/server/handlers/metadata.cc Outdated Show resolved Hide resolved
For workloads with a large amount of topics, this topics vector can grow
to be an oversized allocation.

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
@@ -108,8 +108,8 @@ class delete_topics_request_fixture : public redpanda_thread_fixture {
void validate_topic_is_deleteted(const model::topic& tp) {
kafka::metadata_response resp = get_topic_metadata(tp);
auto it = std::find_if(
std::cbegin(resp.data.topics),
std::cend(resp.data.topics),
resp.data.topics.begin(),
Copy link
Member

Choose a reason for hiding this comment

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

This is because fragmented_vector is missing const-qualified versions of begin & end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe you cannot compare const iterator to non const in frag vector, and you can in vector, so I just switch this all to use the non constant version (in the assertion below)

@piyushredpanda piyushredpanda merged commit 2817299 into redpanda-data:dev Dec 19, 2023
20 checks passed
@rockwotj rockwotj deleted the metadata-alloc branch December 19, 2023 15:45
@vbotbuildovich
Copy link
Collaborator

/backport v23.3.x

@vbotbuildovich
Copy link
Collaborator

/backport v23.2.x

@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v23.2.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-15751-v23.2.x-866 remotes/upstream/v23.2.x
git cherry-pick -x c53565268c217311f115b3be05bdd48402d8aebc

Workflow run logs.

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

Successfully merging this pull request may close these issues.

None yet

7 participants