hachikuji commented on a change in pull request #9881:
URL: https://github.com/apache/kafka/pull/9881#discussion_r557006765



##########
File path: raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java
##########
@@ -325,8 +341,30 @@ private void fireHandleResign() {
     }
 
     @Override
-    public void initialize() throws IOException {
-        quorum.initialize(new OffsetAndEpoch(log.endOffset().offset, 
log.lastFetchedEpoch()));
+    public void initialize(String quorumVoterStrings) throws IOException {

Review comment:
       @jsancio Not really. The one constructor is used in test cases. However, 
the tests could just as well build a `RaftConfig`, which is what I assume we 
would have to do to implement my suggestion above.
   
   > How about storing the RaftConfig as a field of KafkaRaftClient? This would 
allow us to remove a few of the configuration parameter passed through the 
constructor.
   
   I debated suggesting that as well. I decided not to because it seemed vexing 
to do a `Map.get` and parse every time we need the fetch timeout... On the 
other hand, we could move the fields into `RaftConfig`.




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


Reply via email to