Skip to content

Commit 9d9c993

Browse files
[fix][proxy] Only go to connecting state once (#19331)
Relates to: #17831 (comment) ### Motivation When the `ProxyConnection` handles a `Connect` command, that is the time to go to `Connecting` state. There is no other time that makes sense to switch to connecting. The current logic will go to connecting in certain re-authentication scenarios, but those are incorrect. By moving the state change to earlier in the logic, we make the state transition clearer and prevent corrupted state. ### Modifications * Remove `state = State.Connecting` from the `doAuthentication` method, which is called multiple times for various reasons * Add `state = State.Connecting` to the start of the `handleConnect` method. ### Verifying this change The existing tests will verify this change, and reading through the code makes it clear this is a correct change. ### Does this pull request potentially affect one of the following parts: Not a breaking change. ### Documentation - [x] `doc-not-needed` It would be nice to map out the state transitions for our connection classes. That is our of the scope of this small improvement. ### Matching PR in forked repository PR in forked repository: michaeljmarshall#21 (cherry picked from commit c8650ce)
1 parent 6f6130e commit 9d9c993

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -398,12 +398,12 @@ private void doAuthentication(AuthData clientData)
398398
LOG.debug("[{}] Authentication in progress client by method {}.",
399399
remoteAddress, authMethod);
400400
}
401-
state = State.Connecting;
402401
}
403402

404403
@Override
405404
protected void handleConnect(CommandConnect connect) {
406405
checkArgument(state == State.Init);
406+
state = State.Connecting;
407407
this.setRemoteEndpointProtocolVersion(connect.getProtocolVersion());
408408
this.hasProxyToBrokerUrl = connect.hasProxyToBrokerUrl();
409409
this.protocolVersionToAdvertise = getProtocolVersionToAdvertise(connect);

0 commit comments

Comments
 (0)