Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support brokerName in request protocol #3905

Closed
drpmma opened this issue Mar 1, 2022 · 6 comments
Closed

Support brokerName in request protocol #3905

drpmma opened this issue Mar 1, 2022 · 6 comments
Labels
LTS 4.9.x This PR should cherry picked to LTS branch progress/wip Work in progress. Accept or refused to be continue. type/rip

Comments

@drpmma
Copy link
Contributor

drpmma commented Mar 1, 2022

FEATURE REQUEST

  1. Support brokerName in request protocol.
  2. brokerName is a higher-level abstraction than brokerAddress and should be taken in request.
  3. Related components should know request-target by brokerName such as logic queue extension, envoy-like cloud-native gateway, or even more separate computing and storage to decouple computing from current broker logic.
  4. Detail request code: PULL_MESSAGE, UPDATE_CONSUMER_OFFSET, QUERY_CONSUMER_OFFSET, SEARCH_OFFSET_BY_TIMESTAMP, GET_MIN_OFFSET, GET_MAX_OFFSET.
@ni-ze
Copy link
Contributor

ni-ze commented Mar 2, 2022

BrokerAddress can be used as a router, I do not get why do you need brokerName.

@duhenglucky
Copy link
Contributor

duhenglucky commented Mar 2, 2022

@drpmma In favor of this recommendation, the entire Remoting layer is currently IP oriented. Using broker names instead of IP can better hide the underlying state of the broker, especially the abstraction of logical resources brought by logical queues and other features. So would you like to submit a RIP for this proposal?

@duhenglucky duhenglucky added progress/wip Work in progress. Accept or refused to be continue. type/rip labels Mar 2, 2022
@drpmma
Copy link
Contributor Author

drpmma commented Mar 2, 2022

BrokerAddress can be used as a router, I do not get why do you need brokerName.

BrokerAddress is not taken in remoting layer protocol while is just a target. So the secondary components don't have the information to route to other brokers, that's why we need a BrokerName

@lizhanhui
Copy link
Contributor

+1 for the change. IP-centered strategy suffers a lot in the era of cloud and container orchestration. We need logical representation to adapt to this trend.

@chenzlalvin
Copy link
Contributor

Not only brokerName, the queueId also need abstraction, unbind with specific physical node

drpmma added a commit to drpmma/rocketmq that referenced this issue Jun 6, 2022
@drpmma
Copy link
Contributor Author

drpmma commented Jul 15, 2022

Now that bname is supported in RpcRequestHeader, related RPC can use this field to implement.

drpmma added a commit to drpmma/rocketmq that referenced this issue Jul 15, 2022
@zhouxinyu zhouxinyu added the LTS 4.9.x This PR should cherry picked to LTS branch label Sep 20, 2022
drpmma added a commit to drpmma/rocketmq that referenced this issue Sep 22, 2022
drpmma added a commit to drpmma/rocketmq that referenced this issue Sep 22, 2022
drpmma added a commit to drpmma/rocketmq that referenced this issue Sep 26, 2022
lizhanhui pushed a commit that referenced this issue Sep 26, 2022
As this is a backported patch. For the purpose of compatibility, let's merge it as it is.
drpmma added a commit that referenced this issue Oct 17, 2022
 * including `CheckTransactionStateRequestHeader`, `EndTransactionRequestHeader`,
 `ReplyMessageRequestHeader`
drpmma added a commit that referenced this issue Oct 17, 2022
* add bname for `CheckTransactionStateRequestHeader`, `ConsumerSendMsgBackRequestHeader`, `EndTransactionRequestHeader`,
`SendMessageRequestHeader`
zhouxinyu pushed a commit that referenced this issue Oct 17, 2022
 * including `CheckTransactionStateRequestHeader`, `EndTransactionRequestHeader`,
 `ReplyMessageRequestHeader`
aaron-ai pushed a commit that referenced this issue Oct 17, 2022
* [ISSUE #3905] Support bname in protocol for 5.0 client

* add bname for `CheckTransactionStateRequestHeader`, `ConsumerSendMsgBackRequestHeader`, `EndTransactionRequestHeader`,
`SendMessageRequestHeader`

* * add bname for `AckMessageRequestHeader`, `PeekMessageRequestHeader`, `PopMessageRequestHeader`,
    `ChangeInvisibleTimeRequestHeader`, `NotificationRequestHeader` and `PollingInfoRequestHeader`
anuragmadnawat1 pushed a commit to anuragmadnawat1/rocketmq that referenced this issue Nov 2, 2022
…5334)

* [ISSUE apache#3905] Support bname in protocol for 5.0 client

* add bname for `CheckTransactionStateRequestHeader`, `ConsumerSendMsgBackRequestHeader`, `EndTransactionRequestHeader`,
`SendMessageRequestHeader`

* * add bname for `AckMessageRequestHeader`, `PeekMessageRequestHeader`, `PopMessageRequestHeader`,
    `ChangeInvisibleTimeRequestHeader`, `NotificationRequestHeader` and `PollingInfoRequestHeader`
anuragmadnawat1 added a commit to anuragmadnawat1/rocketmq that referenced this issue Nov 2, 2022
…5334) (#16)

* [ISSUE apache#3905] Support bname in protocol for 5.0 client

* add bname for `CheckTransactionStateRequestHeader`, `ConsumerSendMsgBackRequestHeader`, `EndTransactionRequestHeader`,
`SendMessageRequestHeader`

* * add bname for `AckMessageRequestHeader`, `PeekMessageRequestHeader`, `PopMessageRequestHeader`,
    `ChangeInvisibleTimeRequestHeader`, `NotificationRequestHeader` and `PollingInfoRequestHeader`

Co-authored-by: Zhouxiang Zhan <zhouxiang.zzx@alibaba-inc.com>
drpmma added a commit to drpmma/rocketmq that referenced this issue Nov 18, 2022
@drpmma drpmma closed this as completed Nov 22, 2022
drpmma added a commit that referenced this issue Feb 21, 2023
* [ISSUE #3905] Support bname in protocol for 5.0 client

* add bname for `CheckTransactionStateRequestHeader`, `ConsumerSendMsgBackRequestHeader`, `EndTransactionRequestHeader`,
`SendMessageRequestHeader`

* * add bname for `AckMessageRequestHeader`, `PeekMessageRequestHeader`, `PopMessageRequestHeader`,
    `ChangeInvisibleTimeRequestHeader`, `NotificationRequestHeader` and `PollingInfoRequestHeader`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LTS 4.9.x This PR should cherry picked to LTS branch progress/wip Work in progress. Accept or refused to be continue. type/rip
Projects
None yet
Development

No branches or pull requests

6 participants