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

kafka: chunked_vector for config responses #17245

Merged
merged 7 commits into from
Mar 25, 2024

Conversation

pgellert
Copy link
Contributor

This updates the CreateTopics and DescribeConfigs kafka api handlers to use chunked_vector instead of std::vector for the creation of their responses. This is to avoid fragmentation-related issues when the number of topics or partitions is large and so large allocations are necessary to generate the response.

Closes https://github.com/redpanda-data/core-internal/issues/1174

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

Release Notes

  • none

This is necessary to allow using chunked_vector with these encoder
methods.

This continues the work started in the following reference PR:
redpanda-data#8469
This changes two cases where the topics response was copied but instead
could be moved.

This is necessary to enable us to switch to using chunked_vector instead
of std::vector, which only supports move semantics, not copy.
dotnwat
dotnwat previously approved these changes Mar 21, 2024
@dotnwat
Copy link
Member

dotnwat commented Mar 21, 2024

Checking 52 includes from 9 modules against 52 exported files
Traceback (most recent call last):
  File "/home/runner/work/redpanda/redpanda/tools/lint-includes", line 174, in <module>
    run(root)
  File "/home/runner/work/redpanda/redpanda/tools/lint-includes", line 167, in run
    assert not violations, f"Non-exported include found: {violations}"
AssertionError: Non-exported include found: {PosixPath('container/include/container/fragmented_vector.h')}

Looking. Could be a bug in the script

@pgellert
Copy link
Contributor Author

Force-pushed to fix the includes highlighted by the linter

@dotnwat
Copy link
Member

dotnwat commented Mar 21, 2024

Oh cool, thanks. Looks like the linter was working correctly!

dotnwat
dotnwat previously approved these changes Mar 21, 2024
@pgellert
Copy link
Contributor Author

Yep, very useful 🙂

@pgellert
Copy link
Contributor Author

Force-pushed to make the unit tests compile

@pgellert pgellert requested a review from dotnwat March 22, 2024 16:42
@dotnwat
Copy link
Member

dotnwat commented Mar 23, 2024

/ci-repeat 1

@pgellert pgellert merged commit be3c604 into redpanda-data:dev Mar 25, 2024
18 checks passed
@michael-redpanda
Copy link
Contributor

/backport v23.3.x

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

4 participants