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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
