Skip to content

Commit 5beef1a

Browse files
committed
Replace AtomicReference with synchronized in VirtualClusterLifecycleManager
The AtomicReference approach had a race between state.get() and state.updateAndGet() where another thread could advance the state, causing incorrect log messages. Using synchronized makes the read-transition-log sequence atomic. This is sufficient for the single-shot proxy lifecycle where contention is not a concern. Also adjusts log levels: INFO for significant states (Serving, Failed, Stopped), DEBUG for intermediate transitions (Draining). Closes kroxylicious#3696 Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
1 parent 53e00f3 commit 5beef1a

1 file changed

Lines changed: 11 additions & 12 deletions

File tree

kroxylicious-runtime/src/main/java/io/kroxylicious/proxy/internal/VirtualClusterLifecycleManager.java

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
package io.kroxylicious.proxy.internal;
88

99
import java.util.Objects;
10-
import java.util.concurrent.atomic.AtomicReference;
1110
import java.util.function.UnaryOperator;
1211

1312
import org.slf4j.Logger;
@@ -22,15 +21,17 @@
2221
/**
2322
* Manages the lifecycle state of a single virtual cluster.
2423
* <p>
25-
* Thread-safe: state transitions are performed atomically via {@link AtomicReference}.
24+
* Thread-safe: state transitions are performed under synchronization so that the
25+
* read-transition-log sequence is atomic. This is sufficient for the single-shot
26+
* proxy lifecycle where contention is not a concern.
2627
* </p>
2728
*/
2829
public class VirtualClusterLifecycleManager {
2930

3031
private static final Logger LOGGER = LoggerFactory.getLogger(VirtualClusterLifecycleManager.class);
3132

3233
private final String clusterName;
33-
private final AtomicReference<VirtualClusterLifecycleState> state = new AtomicReference<>(new Initializing());
34+
private VirtualClusterLifecycleState state = new Initializing();
3435

3536
public VirtualClusterLifecycleManager(String clusterName) {
3637
this.clusterName = Objects.requireNonNull(clusterName);
@@ -100,24 +101,22 @@ public void stop() {
100101
});
101102
}
102103

103-
public VirtualClusterLifecycleState getState() {
104-
return state.get();
104+
public synchronized VirtualClusterLifecycleState getState() {
105+
return state;
105106
}
106107

107108
public String getClusterName() {
108109
return clusterName;
109110
}
110111

111-
private void transition(UnaryOperator<VirtualClusterLifecycleState> transitionFn) {
112-
// Use get() then updateAndGet() rather than getAndUpdate() so we capture
113-
// both the previous and new state without re-applying the transition function.
114-
VirtualClusterLifecycleState previous = state.get();
115-
VirtualClusterLifecycleState current = state.updateAndGet(transitionFn);
116-
var logBuilder = (current instanceof Failed) ? LOGGER.atWarn() : LOGGER.atInfo();
112+
private synchronized void transition(UnaryOperator<VirtualClusterLifecycleState> transitionFn) {
113+
VirtualClusterLifecycleState previous = state;
114+
state = transitionFn.apply(state);
115+
var logBuilder = (state instanceof Serving || state instanceof Failed || state instanceof Stopped) ? LOGGER.atInfo() : LOGGER.atDebug();
117116
logBuilder
118117
.addKeyValue("virtualCluster", clusterName)
119118
.addKeyValue("from", previous.getClass().getSimpleName())
120-
.addKeyValue("to", current.getClass().getSimpleName())
119+
.addKeyValue("to", state.getClass().getSimpleName())
121120
.log("Virtual cluster lifecycle transition");
122121
}
123122

0 commit comments

Comments
 (0)