Parkerhiphop commented on code in PR #21244:
URL: https://github.com/apache/kafka/pull/21244#discussion_r2661515012
##########
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:
##########
@@ -5527,7 +5537,7 @@ class ReplicaManagerTest {
replicaManager.alterReplicaLogDirs(Map(tp ->
newReplicaFolder.getAbsolutePath))
// Prevent promotion of future replica
-
doReturn(false).when(spiedPartition).maybeReplaceCurrentWithFutureReplica()
+ blockPromotion.set(true)
Review Comment:
Thanks for the CAS suggestion.
I have addressed the race condition and simplified the logic as suggested:
1. **Execution Order**: Moved `blockPromotion.set(true)`
before`alterReplicaLogDirs` to ensure the flag is set before the background
thread starts.
2. **CAS**: Adopted `compareAndSet(true, false)` within `doAnswer` to
automatically unblock the promotion after the first attempt.
To verify the stability, I ran the following test command with no failures
(also updated in the PR description):
Test Command
```
N=100; I=0; while [ $I -lt $N ] && ./gradlew cleanTest core:test --tests
ReplicaManagerTest -PmaxParallelForks=4 \
; do (( I=$I+1 )); echo "Completed run: $I"; sleep 1; done
```
Test Result
```
BUILD SUCCESSFUL in 12s
151 actionable tasks: 2 executed, 149 up-to-date
Consider enabling configuration cache to speed up this build:
https://docs.gradle.org/9.2.1/userguide/configuration_cache_enabling.html
Completed run: 100
```
##########
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:
##########
@@ -5514,6 +5514,16 @@ class ReplicaManagerTest {
try {
val spiedPartition = spy(Partition(tpId, time, replicaManager))
+
+ val blockPromotion = new AtomicBoolean(false)
Review Comment:
Thanks @gaurav-narula for the explanation on visibility!
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
