showuon commented on a change in pull request #10749:
URL: https://github.com/apache/kafka/pull/10749#discussion_r638513347
##########
File path: raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java
##########
@@ -372,27 +371,23 @@ private void maybeFireLeaderChange() {
@Override
public void initialize() {
- try {
- quorum.initialize(new OffsetAndEpoch(log.endOffset().offset,
log.lastFetchedEpoch()));
+ quorum.initialize(new OffsetAndEpoch(log.endOffset().offset,
log.lastFetchedEpoch()));
- long currentTimeMs = time.milliseconds();
- if (quorum.isLeader()) {
- throw new IllegalStateException("Voter cannot initialize as a
Leader");
- } else if (quorum.isCandidate()) {
- onBecomeCandidate(currentTimeMs);
- } else if (quorum.isFollower()) {
- onBecomeFollower(currentTimeMs);
- }
+ long currentTimeMs = time.milliseconds();
+ if (quorum.isLeader()) {
+ throw new IllegalStateException("Voter cannot initialize as a
Leader");
+ } else if (quorum.isCandidate()) {
+ onBecomeCandidate(currentTimeMs);
+ } else if (quorum.isFollower()) {
+ onBecomeFollower(currentTimeMs);
+ }
- // When there is only a single voter, become candidate immediately
- if (quorum.isVoter()
+ // When there is only a single voter, become candidate immediately
+ if (quorum.isVoter()
&& quorum.remoteVoters().isEmpty()
&& !quorum.isCandidate()) {
Review comment:
indent is not correct here.
##########
File path: raft/src/main/java/org/apache/kafka/raft/QuorumState.java
##########
@@ -116,7 +117,7 @@ public void initialize(OffsetAndEpoch logEndOffsetAndEpoch)
throws IOException,
if (election == null) {
election = ElectionState.withUnknownLeader(0, voters);
}
- } catch (final IOException e) {
+ } catch (final Exception e) {
Review comment:
Why should we change `IOException` into `Exception` here?
##########
File path: raft/src/test/java/org/apache/kafka/raft/QuorumStateTest.java
##########
@@ -945,9 +945,7 @@ public void testObserverUnattachedToFollower() throws
IOException {
}
@Test
- public void testInitializeWithCorruptedStore() throws IOException {
- QuorumStateStore stateStore = Mockito.mock(QuorumStateStore.class);
-
Mockito.doThrow(IOException.class).when(stateStore).readElectionState();
Review comment:
Why should we remove these 2 lines? After this PR, we should throw
`UncheckedIOException` in corrupted store, is that right?
##########
File path: raft/src/main/java/org/apache/kafka/raft/FileBasedStateStore.java
##########
@@ -91,14 +92,17 @@ private QuorumStateData readStateFromFile(File file) throws
IOException {
final short dataVersion = dataVersionNode.shortValue();
return QuorumStateDataJsonConverter.read(dataObject, dataVersion);
+ } catch (IOException e) {
+ throw new UncheckedIOException(
+ String.format("Error while reading the Quorum status from
the file %s",file),e);
Review comment:
there should be a space after a comma:
```
String.format("Error while reading the Quorum status from the file %s",
file), e);
```
Also, the indent is not correct here. Please fix it. And same to other
places.
Thanks.
--
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
