[GitHub] [kafka] rhauch commented on a change in pull request #9347: KAFKA-10531: Check for negative values to Thread.sleep call

[GitBox]

rhauch commented on a change in pull request #9347:
URL: https://github.com/apache/kafka/pull/9347#discussion_r499023599



##########
File path: 
connect/runtime/src/main/java/org/apache/kafka/connect/util/KafkaBasedLog.java
##########
@@ -70,7 +70,7 @@
  */
 public class KafkaBasedLog<K, V> {
     private static final Logger log = 
LoggerFactory.getLogger(KafkaBasedLog.class);
-    private static final long CREATE_TOPIC_TIMEOUT_MS = 30000;
+    private static final long CREATE_TOPIC_TIMEOUT_NS = 
TimeUnit.NANOSECONDS.convert(30, TimeUnit.SECONDS);

Review comment:
       We are often using this pattern in other places for time-related 
constants:
   ```suggestion
       private static final long CREATE_TOPIC_TIMEOUT_NS = 
TimeUnit.SECONDS.toNanos(30);
   ```

##########
File path: 
connect/runtime/src/main/java/org/apache/kafka/connect/util/KafkaBasedLog.java
##########
@@ -133,11 +133,14 @@ public void start() {
         List<TopicPartition> partitions = new ArrayList<>();
 
         // We expect that the topics will have been created either manually by 
the user or automatically by the herder
-        List<PartitionInfo> partitionInfos = null;
-        long started = time.milliseconds();
-        while (partitionInfos == null && time.milliseconds() - started < 
CREATE_TOPIC_TIMEOUT_MS) {
+        List<PartitionInfo> partitionInfos = consumer.partitionsFor(topic);
+        long started = time.nanoseconds();
+        long maxSleepMs = 1_000;

Review comment:
       Let's define a constant above:
   ```
   private static final long MAX_SLEEP_MS = TimeUnit.SECONDS.toMillis(1);
   ```
   and then we can replace `maxSleepMs` with `MAX_SLEEP_MS`.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org