ahuang98 commented on code in PR #18987:
URL: https://github.com/apache/kafka/pull/18987#discussion_r1970761435
##########
raft/src/test/java/org/apache/kafka/raft/RaftEventSimulationTest.java:
##########
@@ -1127,14 +1331,75 @@ private MajorityReachedHighWatermark(Cluster cluster) {
@Override
public void verify() {
- cluster.leaderHighWatermark().ifPresent(highWatermark -> {
- long numReachedHighWatermark =
cluster.nodes.entrySet().stream()
- .filter(entry ->
cluster.voters.containsKey(entry.getKey()))
- .filter(entry -> entry.getValue().log.endOffset().offset()
>= highWatermark)
- .count();
- assertTrue(
- numReachedHighWatermark >= cluster.majoritySize(),
- "Insufficient nodes have reached current high watermark");
+ if (cluster.withKip853) {
+ /*
+ * For clusters running in KIP-853 mode, we check that a
majority of at least one of:
+ * 1. the leader's voter set at the HWM
Review Comment:
I see, the sentence continues on L1339
wonder if something like the following might be more clear? maybe you can
fix my language a bit too
```suggestion
* For clusters running in KIP-853 mode, we consider two
possibilities for what the majority of the cluster is:
* 1. the majority based off the current value of
lastVoterSet()
* 2. the majority based off what the leader's voter set was
at the HWM (i.e. the prior lastVoterSet() value)
* This is because verify() may be called after the leader's
in-memory voter set was updated but before the voter change was committed. A
perfect invariant checker would use what has been committed to the log but that
information is not readily exposed for testing.
```
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
