Skip to content

Conversation

@congbobo184
Copy link
Contributor

Motivation

now #11357 has merged, #11357 use SUCCESS command handle tcClientConnectRequest, This is not conducive to later expansion. So add a individual response for CommandTcClientConnectRequest

implement

add command

message CommandTcClientConnectResponse {
    required uint64 request_id = 1;
    optional ServerError error  = 2;
    optional string message     = 3;
}

In order to ensure that the new client is compatible with the old broker, I update protocol version to 19.

    v19 = 19; // Add CommandTcClientConnectRequest and CommandTcClientConnectResponse

if broker protocol version > 18 we should send TcClientConnectCommand
if broker protocol version <= 18 we don't need to send TcClientConnectCommand

Does this pull request potentially affect one of the following parts:
If yes was chosen, please highlight the changes

Dependencies (does it add or upgrade a dependency): (no)
The public API: (no)
The schema: (no)
The default values of configurations: (no)
The wire protocol: (yes)
The rest endpoints: (no)
The admin cli options: (no)
Anything that affects deployment: (no)

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

cnx.channel().close();
}
// if broker protocol version < 19, don't send TcClientConnectRequest to broker.
if (cnx.getRemoteEndpointProtocolVersion() > ProtocolVersion.v18.getValue()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is great

@congbobo184 congbobo184 merged commit e29f720 into apache:master Oct 9, 2021
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
…mand. (apache#12301)

## Motivation
now apache#11357 has merged, apache#11357 use `SUCCESS` command handle tcClientConnectRequest, This is not conducive to later expansion. So add a individual response for `CommandTcClientConnectRequest`

## implement
add command 
```
message CommandTcClientConnectResponse {
    required uint64 request_id = 1;
    optional ServerError error  = 2;
    optional string message     = 3;
}
```

In order to ensure that the new client is compatible with the old broker, I update protocol version to 19.

```
    v19 = 19; // Add CommandTcClientConnectRequest and CommandTcClientConnectResponse
```
if broker protocol version > 18 we should send TcClientConnectCommand
if broker protocol version <= 18 we don't need to send TcClientConnectCommand
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants