cmccabe commented on a change in pull request #10705:
URL: https://github.com/apache/kafka/pull/10705#discussion_r633917057
##########
File path: raft/src/main/java/org/apache/kafka/raft/RaftClient.java
##########
@@ -56,24 +57,17 @@
void handleSnapshot(SnapshotReader<T> reader);
/**
Review comment:
It's fine to combine these APIs, but we need to document what happens if
the current leader resigns, but we don't know who the new leader is yet. Do we
get passed a LeaderAndEpoch with the current epoch + 1 and a node ID of -1? If
so, do we then expect to see another LeaderAndEpoch with the current epoch + 1
and a valid node -1?
In other words, let's say node 0 is the leader and then resigns, and then
node 1 becomes the leader. Does it look like this:
```
handleLeaderChange(LeaderAndEpoch(nodeId=0, epoch=0))
handleLeaderChange(LeaderAndEpoch(nodeId=-1, epoch=1))
handleLeaderChange(LeaderAndEpoch(nodeId=1, epoch=1))
```
Or would you rather have something like this?
```
handleLeaderChange(LeaderAndEpoch(nodeId=0, epoch=0))
handleLeaderChange(LeaderAndEpoch(nodeId=-1, epoch=0))
handleLeaderChange(LeaderAndEpoch(nodeId=1, epoch=1))
```
It seems like the second one will break a lot of invariants, so probably
should be avoided. The first one might break some invariants too, though. We'd
have to look.
Or you could choose to burn an epoch like this:
```
handleLeaderChange(LeaderAndEpoch(nodeId=0, epoch=0))
handleLeaderChange(LeaderAndEpoch(nodeId=-1, epoch=1))
handleLeaderChange(LeaderAndEpoch(nodeId=1, epoch=2))
```
Given that we only have a 31-bit epoch in the first place, that seems
unwise, though.
--
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
