You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "bbejeck (via GitHub)" <gi...@apache.org> on 2026/09/24 23:18:27 UTC
[PR] MINOR: Update test for review comments [kafka]
bbejeck opened a new pull request, #23586:
URL: https://github.com/apache/kafka/pull/23586
This PR applies comments on the integration test from #23570
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Update test for review comments [kafka]
Posted by "bbejeck (via GitHub)" <gi...@apache.org>.
bbejeck commented on code in PR #23586:
URL: https://github.com/apache/kafka/pull/23586#discussion_r4136982086
##########
streams/integration-tests/src/test/java/org/apache/kafka/streams/integration/SourceTopicChangelogRestoreIntegrationTest.java:
##########
@@ -109,7 +109,8 @@ public void setUp(final TestInfo testInfo) throws Exception {
for (final String key : preloadedKeys()) {
vehicles.add(KeyValue.pair(key, "initial-state"));
}
- produce(TABLE_TOPIC, vehicles);
+ // batch the preload so the 5000 records load quickly (see produce())
+ produce(TABLE_TOPIC, vehicles, false);
Review Comment:
Good catch, the preload is what the restore consumer reads back, so batching it lets the restore get through it in a handful of fetches. I reverted to one record per batch everywhere. The test still fails without the fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Update test for review comments [kafka]
Posted by "bbejeck (via GitHub)" <gi...@apache.org>.
bbejeck merged PR #23586:
URL: https://github.com/apache/kafka/pull/23586
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Update test for review comments [kafka]
Posted by "lucasbru (via GitHub)" <gi...@apache.org>.
lucasbru commented on code in PR #23586:
URL: https://github.com/apache/kafka/pull/23586#discussion_r4124378499
##########
streams/integration-tests/src/test/java/org/apache/kafka/streams/integration/SourceTopicChangelogRestoreIntegrationTest.java:
##########
@@ -239,12 +249,20 @@ private Properties streamsConfig(final String clientId) {
}
private void produce(final String topic, final List<KeyValue<String, String>> records) throws Exception {
+ produce(topic, records, true);
+ }
+
+ private void produce(final String topic,
Review Comment:
The oneRecordPerBatch flag makes the `false` at the call site a bit opaque without the comment. Maybe clearer as a separately named method (e.g. preload or produceBatched) for the one case that needs it, instead of a boolean?
##########
streams/integration-tests/src/test/java/org/apache/kafka/streams/integration/SourceTopicChangelogRestoreIntegrationTest.java:
##########
@@ -109,7 +109,8 @@ public void setUp(final TestInfo testInfo) throws Exception {
for (final String key : preloadedKeys()) {
vehicles.add(KeyValue.pair(key, "initial-state"));
}
- produce(TABLE_TOPIC, vehicles);
+ // batch the preload so the 5000 records load quickly (see produce())
+ produce(TABLE_TOPIC, vehicles, false);
Review Comment:
TABLE_TOPIC is also the changelog that gets restored later under fault injection. With normal batching here, doesn't the preload risk landing as a handful of large batches on disk? Kafka always returns at least one full batch per fetch even over max.partition.fetch.bytes, so the delayed restore consumer could get through the preloaded records in far fewer, larger fetches than before, instead of the many small ones the restore-fetch-delay design seems to assume. Could that shrink the stall window the corruption scenario depends on?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Update test for review comments [kafka]
Posted by "bbejeck (via GitHub)" <gi...@apache.org>.
bbejeck commented on PR #23586:
URL: https://github.com/apache/kafka/pull/23586#issuecomment-5896411261
@lucasbru thanks for the review - I've addressed the comments and it's ready for another review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Update test for review comments [kafka]
Posted by "bbejeck (via GitHub)" <gi...@apache.org>.
bbejeck commented on code in PR #23586:
URL: https://github.com/apache/kafka/pull/23586#discussion_r4136995187
##########
streams/integration-tests/src/test/java/org/apache/kafka/streams/integration/SourceTopicChangelogRestoreIntegrationTest.java:
##########
@@ -239,12 +249,20 @@ private Properties streamsConfig(final String clientId) {
}
private void produce(final String topic, final List<KeyValue<String, String>> records) throws Exception {
+ produce(topic, records, true);
+ }
+
+ private void produce(final String topic,
Review Comment:
deleted with the comments addressed above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org