-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Deal with topic recreation for producer and consumer (using metadata) #5022
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
base: master
Are you sure you want to change the base?
Conversation
🎉 All Contributor License Agreements have been signed. Ready to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 6 out of 8 changed files in this pull request and generated 1 comment.
Files not reviewed (2)
- tests/CMakeLists.txt: Language not supported
- win32/tests/tests.vcxproj: Language not supported
Comments suppressed due to low confidence (1)
src/rdkafka_topic.c:1010
- The variable 'is_idempodent' is misspelled; please rename it to 'is_idempotent' for clarity.
if (is_idempodent && rd_kafka_pid_valid(rktp->rktp_eos.pid) && !topic_id_change)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 7 out of 9 changed files in this pull request and generated no comments.
Files not reviewed (2)
- tests/CMakeLists.txt: Language not supported
- win32/tests/tests.vcxproj: Language not supported
Comments suppressed due to low confidence (1)
src/rdkafka_topic.c:1044
- The variable 'is_idempodent' appears to be misspelled. It should likely be renamed to 'is_idempotent' for consistency.
if (is_idempodent && rd_kafka_pid_valid(rktp->rktp_eos.pid) && !topic_id_change)
14ab6c5
to
1b6b2f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 8 out of 10 changed files in this pull request and generated no comments.
Files not reviewed (2)
- tests/CMakeLists.txt: Language not supported
- win32/tests/tests.vcxproj: Language not supported
Comments suppressed due to low confidence (2)
src/rdkafka_topic.c:1044
- Possible typo in variable name 'is_idempodent'; consider renaming it to 'is_idempotent' for clarity.
if (is_idempodent && rd_kafka_pid_valid(rktp->rktp_eos.pid) && !topic_id_change)
tests/0107-topic_recreate.c:325
- The producer handle 'rk_producer' is NULL at this point; ensure a valid handle is provided when calling test_delete_topic.
test_delete_topic(rk_producer, topic);
There is a bunch of things we need to do for producers when topics are recreated (topic id changes). Unfortunately we cannot do it all because the Produce RPC doesn't yet use topic ids. We can make a best-effort from metadata requests.
Similar to produce, fetch also has to be changed. We make these changes from metadata, luckily fetch has UNKNOWN_TOPIC_ID errors, so that helps us be more deterministic rather than best-effort.
Here's a summary of changes:
This adds mock broker changes, mock tests, and real broker tests (only for consumer, it's not yet possible to have real-broker tests for producer as it's not using topic ids on the broker)