[CORE-13383] Implement KIP-709: offset fetch v8#27794
[CORE-13383] Implement KIP-709: offset fetch v8#27794BenPope merged 9 commits intoredpanda-data:devfrom
Conversation
Signed-off-by: Ben Pope <ben@redpanda.com>
There was a problem hiding this comment.
Pull Request Overview
This PR implements KIP-709 for offset fetch version 8, enabling support for fetching offsets for multiple consumer groups in a single request. Currently, the implementation runs tests against version 7 to verify existing functionality isn't broken.
Key changes:
- Extends the offset fetch protocol to support multiple groups in a single request
- Updates data structures to use chunked_vector for better memory efficiency
- Adds new protocol schema definitions for v8 request/response structures
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/v/pandaproxy/json/requests/offset_fetch.h |
Updates return type to use chunked_vector |
src/v/kafka/server/tests/offset_fetch_test.cc |
Adds comprehensive test coverage for all supported versions including v8+ |
src/v/kafka/server/tests/BUILD |
Adds dependency for server code in tests |
src/v/kafka/server/server.cc |
Core implementation of v8 protocol with request/response conversion logic |
src/v/kafka/server/group_router.h |
Updates method signature to pass by value |
src/v/kafka/server/group_router.cc |
Implements multi-group routing logic |
src/v/kafka/server/group_manager.h |
Updates method signature |
src/v/kafka/server/group_manager.cc |
Implements multi-group offset fetch handling |
src/v/kafka/server/group.h |
Updates method signature and return type |
src/v/kafka/server/group.cc |
Implements single-group offset fetch logic |
src/v/kafka/protocol/schemata/offset_fetch_response.json |
Schema definition for v8 response |
src/v/kafka/protocol/schemata/offset_fetch_request.json |
Schema definition for v8 request |
src/v/kafka/protocol/schemata/generator.py |
Updates code generation for new structures |
src/v/kafka/protocol/offset_fetch.h |
Updates response construction helpers |
src/v/kafka/client/test/consumer_group.cc |
Updates client test code |
src/v/kafka/client/consumer.h |
Updates consumer interface |
src/v/kafka/client/consumer.cc |
Updates consumer implementation |
src/v/kafka/client/client.h |
Updates client interface |
src/v/kafka/client/client.cc |
Updates client implementation |
BenPope
force-pushed
the
core-13383-kip-709-offset-fetch-v8
branch
2 times, most recently
from
4152bac to
f3fa1ac
Compare
Retry command for Build#73193please wait until all jobs are finished before running the slash command |
|
/ci-repeat 1 |
Retry command for Build#73226please wait until all jobs are finished before running the slash command |
Pin cluster_link to v7 for now, since that's what it supports. Signed-off-by: Ben Pope <ben@redpanda.com>
No functional change. * Make the constructors explicit * Take the sink parameter by value, to make the callsite obvious * Extract get_topics to a general function Signed-off-by: Ben Pope <ben@redpanda.com>
BenPope
force-pushed
the
core-13383-kip-709-offset-fetch-v8
branch
2 times, most recently
from
c8f19a1 to
2e1cb0a
Compare
BenPope
force-pushed
the
core-13383-kip-709-offset-fetch-v8
branch
from
2e1cb0a to
ceb254c
Compare
| if (api_version < version_with_groups) { | ||
| check_response_topics(resp.data, all_topics); | ||
| } else { | ||
| BOOST_REQUIRE_EQUAL(resp.data.groups.size(), 1); |
There was a problem hiding this comment.
Can you add a test case with >1 groups?
There was a problem hiding this comment.
Can you add a test case with >1 groups?
Done
joe-redpanda
left a comment
joe-redpanda
left a comment
There was a problem hiding this comment.
Minor comments, otherwise looks good
src/v/kafka/server/server.cc
Outdated
| vassert( | ||
| res.data.groups.size() == 1, | ||
| "Only single group fetch is supported in v{}", | ||
| target_version); |
There was a problem hiding this comment.
I would remove this assertion as it depends on user input ?
There was a problem hiding this comment.
I would remove this assertion as it depends on user input ?
It depends on:
- Deserialization
- The logic for offset fetch
It shouldn't be possible to have not 1 group in version <= 8, but I've changed it from an assert anyway.
Signed-off-by: Ben Pope <ben@redpanda.com>
Signed-off-by: Ben Pope <ben@redpanda.com>
No functional change. * Extract functionality into functions. Signed-off-by: Ben Pope <ben@redpanda.com>
No functional change. * Reorder functions Signed-off-by: Ben Pope <ben@redpanda.com>
In preperation for offset_fetch v8, which allows multiple groups to be requested, implement everything as if it were v8. If the incoming request is < v8, then convert the request to latest, and the response to the version that was requested. Signed-off-by: Ben Pope <ben@redpanda.com>
Signed-off-by: Ben Pope <ben@redpanda.com>
BenPope
dismissed stale reviews from mmaslankaprv and joe-redpanda
via
4555fe5
BenPope
force-pushed
the
core-13383-kip-709-offset-fetch-v8
branch
from
ceb254c to
4555fe5
Compare
|
Changes in force-push
|
This PR implements KIP-709 for offset fetch version 8, enabling support for fetching offsets for multiple consumer groups in a single request. Currently, the implementation runs tests against version 7 to verify existing functionality isn't broken.
Key changes:
Backports Required
Release Notes
Features