C0urante commented on code in PR #13178:
URL: https://github.com/apache/kafka/pull/13178#discussion_r1100676199
##########
connect/mirror/src/test/java/org/apache/kafka/connect/mirror/integration/MirrorConnectorsIntegrationBaseTest.java:
##########
@@ -503,25 +495,6 @@ public void testOffsetSyncsTopicsOnTarget() throws
Exception {
produceMessages(primary, "test-topic-1");
- ReplicationPolicy replicationPolicy = new
MirrorClient(mm2Config.clientConfig(BACKUP_CLUSTER_ALIAS)).replicationPolicy();
- String remoteTopic =
replicationPolicy.formatRemoteTopic(PRIMARY_CLUSTER_ALIAS, "test-topic-1");
-
- // Check offsets are pushed to the checkpoint topic
- Consumer<byte[], byte[]> backupConsumer =
backup.kafka().createConsumerAndSubscribeTo(Collections.singletonMap(
- "auto.offset.reset", "earliest"), PRIMARY_CLUSTER_ALIAS +
".checkpoints.internal");
- waitForCondition(() -> {
- ConsumerRecords<byte[], byte[]> records =
backupConsumer.poll(Duration.ofSeconds(1L));
- for (ConsumerRecord<byte[], byte[]> record : records) {
- Checkpoint checkpoint = Checkpoint.deserializeRecord(record);
- if (remoteTopic.equals(checkpoint.topicPartition().topic())) {
- return true;
- }
- }
- return false;
- }, 30_000,
- "Unable to find checkpoints for " + PRIMARY_CLUSTER_ALIAS +
".test-topic-1"
- );
Review Comment:
Thanks for the explanation, that clears things up nicely.
> Upon further inspection, maybe this test makes sense to turn into a
parameter for other tests, to verify that the functionality of the checkpoints
when the syncs topic is moved around. Does that make sense to do in this PR?
I'm hesitant to drop this coverage from the test, because it seems at least
as useful to verify that offset sync records are published in the expected
location (and translated to consumer offsets) as it is to verify that they
aren't published in the incorrect location. My personal preference would be to
add the minimal additional logic necessary to this test to keep that coverage,
and then pursue parameterization of offset sync location out of band. I'm also
not even sure that parameterization would be worth the tradeoff in build/test
time, although we might be able to find a healthy compromise there.
WDYT?
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
