apoorvmittal10 commented on code in PR #17720:
URL: https://github.com/apache/kafka/pull/17720#discussion_r1834643738


##########
clients/src/main/java/org/apache/kafka/clients/admin/internals/AdminMetadataManager.java:
##########
@@ -289,17 +304,24 @@ public void update(Cluster cluster, long now) {
 
         this.state = State.QUIESCENT;
         this.fatalException = null;
+        this.metadataAttemptStartMs = Optional.empty();
 
         if (!cluster.nodes().isEmpty()) {
             this.cluster = cluster;
         }
     }
 
+    public void initiateRebootstrap() {
+        requestUpdate();
+        this.metadataAttemptStartMs = Optional.of(0L);

Review Comment:
   Question: Why don't we set the time to `now` as we do in NetworkClient?



##########
clients/src/main/java/org/apache/kafka/clients/NetworkClient.java:
##########
@@ -1236,6 +1316,12 @@ public void close() {
             this.metadata.close();
         }
 
+        private void rebootstrap(long now) {
+            closeAll();
+            metadata.rebootstrap();
+            metadataAttemptStartMs = Optional.of(now);

Review Comment:
   Question: Do we need to set the time while rebootstrapping or reset to 
`Optional.empty`? As whenever there would be successful response from metadata 
then the time will be updated?
   
   Or is it more to handle that after re-bootstrap, if we never receive 
successful metadata response then we would like to rebootstrap again?



##########
clients/src/main/java/org/apache/kafka/clients/NetworkClient.java:
##########
@@ -166,11 +169,51 @@ public NetworkClient(Selectable selector,
              time,
              discoverBrokerVersions,
              apiVersions,
-             null,
              logContext,
+             Long.MAX_VALUE,

Review Comment:
   Seems this constructor is used be Connect Worker which gets passed to 
ConsumerNetworkClient. So should we not support the consumer client spun in 
other dependant modules? Do you think the a default value of `Long.MAX_VALUE` 
for this config makes sense so we can just get the ocnfigured vs default value?



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

Reply via email to