rocketmq icon indicating copy to clipboard operation
rocketmq copied to clipboard

[Summer of code] Identify broker by uuid

Open hzh0425 opened this issue 3 years ago • 2 comments

Make sure set the target branch to develop

What is the purpose of the change

Identify broker by uuid

Brief changelog

XX

Verifying this change

XXXX

Follow this checklist to help us incorporate your contribution quickly and easily. Notice, it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR.

  • [x] Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • [x] Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
  • [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • [x] Write necessary unit-test(over 80% coverage) to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
  • [x] Run mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyle to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.
  • [ ] If this contribution is large, please file an Apache Individual Contributor License Agreement.

hzh0425 avatar Jul 23 '22 02:07 hzh0425

@RongtongJin Hi, I open it again

hzh0425 avatar Jul 23 '22 02:07 hzh0425

@hzh0425 I have created a temporary feature/controler-broker-identify branch to solve this issue. You can solve the code conflict first

RongtongJin avatar Aug 10 '22 03:08 RongtongJin

Two questions:

  1. Why uuid?
  2. We already have brokerId, adding a second property named brokerIdentity is very confusing. Give it another name.

lizhanhui avatar Aug 11 '22 05:08 lizhanhui

@RongtongJin how about naming it as "controllerBrokerId", means it is an id for controller mode.

Just like the dledgerSelfId, controllerBrokerId may have some relation to brokerId.

when the broker is master, brokerId will be set to 0. when the broker is slave, brokerId will be set to controllerBrokerId.

the old admin command clusterList could see the value

dongeforever avatar Aug 11 '22 09:08 dongeforever

2. property

Hi @dongeforever , There is already a property similar to controllerBrokerId in org.apache.rocketmq.controller.impl.manager.BrokerInfo#brokerIdTable

The controllerBrokerId is currently allocated according to the IP. If the IP changes, there will be problems. So we hope to have a uuid property that can identify the broker and does not require user configuration. It will also be initialized (if not) when the broker starts and be persisted to the store directory. After that, the identity of the broker will be changed from IP to persistent uuid.

However, after your reminder, we have come up with another solution. If we persist the controllerBrokerId allocated by the controller according to the IP to the local storage of the broker when the first apply, and bring the controllerBrokerId with each request later, it seems that we can also solve the issue of IP change. For the current implementation, such changes will be slightly larger.

Currently, we prefer the second solution. What about you?

RongtongJin avatar Aug 12 '22 02:08 RongtongJin

Obviously, this pull request is sort of haste. The problem trying to solve is not new and there are better ones in the field. A numeric "node-id" suffices as long as we have a clear cluster membership algorithm in hand.

Thinking it out prior to code is always good.

lizhanhui avatar Aug 12 '22 02:08 lizhanhui