You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "dajac (via GitHub)" <gi...@apache.org> on 2025/01/20 17:18:14 UTC
[PR] KAFKA-18487; Remove ReplicaManager#stopReplicas [kafka]
dajac opened a new pull request, #18647:
URL: https://github.com/apache/kafka/pull/18647
This patch removes `ReplicaManager#stopReplicas`. I have ensured that removed unit tests are covered by other existing tests or updated to use kraft.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18487; Remove ReplicaManager#stopReplicas [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #18647:
URL: https://github.com/apache/kafka/pull/18647#issuecomment-2603882097
Resolved the conflicts and removed `StopReplicaPartitionState`.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18487; Remove ReplicaManager#stopReplicas [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #18647:
URL: https://github.com/apache/kafka/pull/18647#issuecomment-2603496337
Conflicts need to be resolved. Once those are done, you should be able to delete the following class in `ReplicaManager`:
> case class StopReplicaPartitionState(leaderEpoch: Int, deletePartition: Boolean)
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18487; Remove ReplicaManager#stopReplicas [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #18647:
URL: https://github.com/apache/kafka/pull/18647#discussion_r1922706349
##########
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:
##########
@@ -2146,101 +2144,6 @@ class ReplicaManagerTest {
}
}
- @Test
- def testClearFetchPurgatoryOnStopReplica(): Unit = {
- // As part of a reassignment, we may send StopReplica to the old leader.
- // In this case, we should ensure that pending purgatory operations are cancelled
- // immediately rather than sitting around to timeout.
-
- val mockTimer = new MockTimer(time)
- val replicaManager = setupReplicaManagerWithMockedPurgatories(mockTimer, aliveBrokerIds = Seq(0, 1))
-
- try {
- val tp0 = new TopicPartition(topic, 0)
- val tidp0 = new TopicIdPartition(topicId, tp0)
- val offsetCheckpoints = new LazyOffsetCheckpoints(replicaManager.highWatermarkCheckpoints.asJava)
- replicaManager.createPartition(tp0).createLogIfNotExists(isNew = false, isFutureReplica = false, offsetCheckpoints, None)
- val partition0Replicas = Seq[Integer](0, 1).asJava
-
- val becomeLeaderRequest = new LeaderAndIsrRequest.Builder(ApiKeys.LEADER_AND_ISR.latestVersion, 0, 0, brokerEpoch,
- Seq(new LeaderAndIsrPartitionState()
- .setTopicName(tp0.topic)
- .setPartitionIndex(tp0.partition)
- .setControllerEpoch(0)
- .setLeader(0)
- .setLeaderEpoch(1)
- .setIsr(partition0Replicas)
- .setPartitionEpoch(0)
- .setReplicas(partition0Replicas)
- .setIsNew(true)).asJava,
- topicIds.asJava,
- Set(new Node(0, "host1", 0), new Node(1, "host2", 1)).asJava).build()
- replicaManager.becomeLeaderOrFollower(1, becomeLeaderRequest, (_, _) => ())
-
- val partitionData = new FetchRequest.PartitionData(Uuid.ZERO_UUID, 0L, 0L, 100,
- Optional.of(1))
- val fetchResult = fetchPartitionAsConsumer(replicaManager, tidp0, partitionData, maxWaitMs = 10)
- assertFalse(fetchResult.hasFired)
- when(replicaManager.metadataCache.contains(tp0)).thenReturn(true)
-
- // We have a fetch in purgatory, now receive a stop replica request and
- // assert that the fetch returns with a NOT_LEADER error
- replicaManager.stopReplicas(2, 0, 0,
- mutable.Map(tp0 -> new StopReplicaPartitionState()
- .setPartitionIndex(tp0.partition)
- .setDeletePartition(true)
- .setLeaderEpoch(LeaderAndIsr.EPOCH_DURING_DELETE)))
-
- assertEquals(Errors.NOT_LEADER_OR_FOLLOWER, fetchResult.assertFired.error)
- } finally {
- replicaManager.shutdown(checkpointHW = false)
- }
- }
-
- @Test
- def testClearProducePurgatoryOnStopReplica(): Unit = {
Review Comment:
This one is covered by testDeltaToFollowerCompletesProduce.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18487; Remove ReplicaManager#stopReplicas [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac merged PR #18647:
URL: https://github.com/apache/kafka/pull/18647
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18487; Remove ReplicaManager#stopReplicas [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18647:
URL: https://github.com/apache/kafka/pull/18647#discussion_r1922703177
##########
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:
##########
@@ -2146,101 +2144,6 @@ class ReplicaManagerTest {
}
}
- @Test
Review Comment:
Since you have already checked, it would be helpful to name the Kraft version of the test.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18487; Remove ReplicaManager#stopReplicas [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #18647:
URL: https://github.com/apache/kafka/pull/18647#discussion_r1922706139
##########
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:
##########
@@ -2146,101 +2144,6 @@ class ReplicaManagerTest {
}
}
- @Test
Review Comment:
It is covered by testDeltaToFollowerCompletesFetch.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org