KAFKA-18276 Migrate RebootstrapTest to new test infra #19046
Conversation
github-actions
bot
added
triage
| public void testRebootstrap(ClusterInstance clusterInstance) throws ExecutionException, InterruptedException { | ||
| String topic = "topic"; | ||
| try (Admin admin = clusterInstance.admin()) { | ||
| admin.createTopics(Collections.singletonList(new NewTopic(topic, BROKER_COUNT, (short) 2))).all().get(); |
There was a problem hiding this comment.
Please use List.of() instead Collections.singletonList, and I don't think we need to get the future value.
admin.createTopics(List.of(new NewTopic(topic, BROKER_COUNT, (short) 2)));
There was a problem hiding this comment.
Thank you for your comments.
Sure, we don't need to get any future value.
| @ClusterTemplate(value = "generator") | ||
| public void testRebootstrap(ClusterInstance clusterInstance) throws ExecutionException, InterruptedException { | ||
| String topic = "topic"; | ||
| try (Admin admin = clusterInstance.admin()) { |
There was a problem hiding this comment.
nit: Variables should be declared using var to maintain consistency throughout the file.
There was a problem hiding this comment.
I've fixed it to maintain consistency by using var.
|
|
||
| try (var producer = clusterInstance.producer()) { | ||
| // Only the server 0 is available for the producer during the bootstrap. | ||
| RecordMetadata recordMetadata0 = producer.send(new ProducerRecord<>(topic, part, "key 1".getBytes(), "value 1".getBytes())).get(); |
There was a problem hiding this comment.
ditto: Variables should be declared using var to maintain consistency throughout the file.
There was a problem hiding this comment.
I've fixed it to maintain consistency by using var.
| // Current server 0 is offline. | ||
| // However, the server 1 from the bootstrap list is online. | ||
| // Should be able to produce records. | ||
| RecordMetadata recordMetadata1 = producer.send(new ProducerRecord<>(topic, part, "key 1".getBytes(), "value 1".getBytes())).get(); |
There was a problem hiding this comment.
ditto: Variables should be declared using var to maintain consistency throughout the file.
There was a problem hiding this comment.
I've fixed it to maintain consistency by using var.
| server0.startup(); | ||
|
|
||
| // The same situation, but the server 1 has gone and server 0 is back. | ||
| RecordMetadata recordMetadata2 = producer.send(new ProducerRecord<>(topic, part, "key 1".getBytes(), "value 1".getBytes())).get(); |
There was a problem hiding this comment.
ditto: Variables should be declared using var to maintain consistency throughout the file.
There was a problem hiding this comment.
I've fixed it to maintain consistency by using var.
| KafkaBroker server0 = clusterInstance.brokers().get(0); | ||
| KafkaBroker server1 = clusterInstance.brokers().get(1); |
There was a problem hiding this comment.
ditto: Variables should be declared using var to maintain consistency throughout the file.
There was a problem hiding this comment.
I've fixed it to maintain consistency by using var.
| properties.put("metadata.recovery.rebootstrap.trigger.ms", "5000"); | ||
| } else { | ||
| properties.put("metadata.recovery.rebootstrap.trigger.ms", "3600000"); | ||
| properties.put("socket.connection.setup.timeout.ms", "5000"); | ||
| properties.put("socket.connection.setup.timeout.max.ms", "5000"); | ||
| properties.put("reconnect.backoff.ms", "1000"); | ||
| properties.put("reconnect.backoff.max.ms", "1000"); | ||
| } | ||
| properties.put("metadata.recovery.strategy", "rebootstrap"); |
There was a problem hiding this comment.
We could use CommonClientConfigs.METADATA_RECOVERY_REBOOTSTRAP_TRIGGER_MS_CONFIG instead of plaintext, other configs are same.
There was a problem hiding this comment.
I can't do it because the styleAction check blocks the reference byCommonClientConfigs in tests.
I only just wrote it in plaintext.
There was a problem hiding this comment.
You can relax the conditions for core module import-control.
There was a problem hiding this comment.
Thank you, the method effectively relaxes the conditions.
I have tested to update the org.apache.kafka.clients pkg in import-control-core.xml.
Is it okay to submit this change directly on commit?
There was a problem hiding this comment.
I think it's not a problem.
Reason: `List.of` optimizing for small size and catch bugs easier.
…ce declared variables
Migrate RebootstrapTest to new test infra and remove the old Scala test.
The test results
