ahuang98 commented on code in PR #14206:
URL: https://github.com/apache/kafka/pull/14206#discussion_r1339228692
##########
metadata/src/test/java/org/apache/kafka/image/TopicsImageTest.java:
##########
@@ -176,22 +200,30 @@ private PartitionRegistration newPartition(int[]
replicas) {
public void testBasicLocalChanges() {
int localId = 3;
/* Changes already include in DELTA1_RECORDS and IMAGE1:
- * foo - topic id deleted
+ * foo - topic id deleted then recreated with different topic id
* bar-0 - stay as follower with different partition epoch
* baz-0 - new topic to leader
+ * bam - topic id created then deleted
*/
List<ApiMessageAndVersion> topicRecords = new
ArrayList<>(DELTA1_RECORDS);
- // Create a new foo topic with a different id
- Uuid newFooId = Uuid.fromString("b66ybsWIQoygs01vdjH07A");
+ // Attempt to create a new foo topic with a different id and replica
placement
+ topicRecords.add(
+ new ApiMessageAndVersion(
+ new TopicRecord().setName("foo").setTopicId(FOO_UUID3),
+ TOPIC_RECORD.highestSupportedVersion()
+ )
+ );
+ topicRecords.add(newPartitionRecord(FOO_UUID3, 0, Arrays.asList(0, 1,
localId)));
Review Comment:
I had a feeling someone might comment on this 😅 I agree we should never be
able to get to this point (though arguably we might now with the metadata
recovery tool), my intent was to show with this test that we don't do any
validation - if that changed in the future, then the test would be changed as
well.
I see how it's confusing though, maybe best to remove it.
##########
metadata/src/test/java/org/apache/kafka/image/TopicsImageTest.java:
##########
@@ -176,22 +200,30 @@ private PartitionRegistration newPartition(int[]
replicas) {
public void testBasicLocalChanges() {
int localId = 3;
/* Changes already include in DELTA1_RECORDS and IMAGE1:
- * foo - topic id deleted
+ * foo - topic id deleted then recreated with different topic id
* bar-0 - stay as follower with different partition epoch
* baz-0 - new topic to leader
+ * bam - topic id created then deleted
*/
List<ApiMessageAndVersion> topicRecords = new
ArrayList<>(DELTA1_RECORDS);
- // Create a new foo topic with a different id
- Uuid newFooId = Uuid.fromString("b66ybsWIQoygs01vdjH07A");
+ // Attempt to create a new foo topic with a different id and replica
placement
+ topicRecords.add(
+ new ApiMessageAndVersion(
+ new TopicRecord().setName("foo").setTopicId(FOO_UUID3),
+ TOPIC_RECORD.highestSupportedVersion()
+ )
+ );
+ topicRecords.add(newPartitionRecord(FOO_UUID3, 0, Arrays.asList(0, 1,
localId)));
Review Comment:
I had a feeling someone might comment on this 😅 I agree we should never be
able to get to this point (though arguably we might now with the metadata
recovery tool), my intent was to show with this test that we don't do any
validation - if that changed in the future, then the test would be changed as
well.
I see how it's confusing though, maybe best to remove it.
--
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
