You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/08/08 15:52:14 UTC
[GitHub] [rocketmq] mxsm opened a new pull request, #4798: [ISSUE #4791] Add elect master command for admin CLI
mxsm opened a new pull request, #4798:
URL: https://github.com/apache/rocketmq/pull/4798
**Make sure set the target branch to `develop`**
## What is the purpose of the change
close #4791
## Brief changelog
- Add elect master command for admin CLI
## 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](https://github.com/apache/rocketmq/issues) 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](https://github.com/apache/rocketmq/tree/master/test).
- [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](http://www.apache.org/licenses/#clas).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] tsunghanjacktsai commented on a diff in pull request #4798: [ISSUE #4791] Add elect master command for admin CLI
Posted by GitBox <gi...@apache.org>.
tsunghanjacktsai commented on code in PR #4798:
URL: https://github.com/apache/rocketmq/pull/4798#discussion_r941942077
##########
tools/src/main/java/org/apache/rocketmq/tools/command/controller/ReElectMasterSubCommand.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.tools.command.controller;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.rocketmq.common.protocol.body.BrokerMemberGroup;
+import org.apache.rocketmq.common.protocol.header.namesrv.controller.ElectMasterResponseHeader;
+import org.apache.rocketmq.remoting.RPCHook;
+import org.apache.rocketmq.tools.admin.DefaultMQAdminExt;
+import org.apache.rocketmq.tools.command.SubCommand;
+import org.apache.rocketmq.tools.command.SubCommandException;
+
+public class ReElectMasterSubCommand implements SubCommand {
+
+ @Override
+ public String commandName() {
+ return "electMaster";
+ }
+
+ @Override
+ public String commandDesc() {
+ return "Re-elect the specified broker as master";
+ }
+
+ @Override
+ public Options buildCommandlineOptions(Options options) {
+ Option opt = new Option("a", "controllerAddress", true, "The address of controller");
+ opt.setRequired(true);
+ options.addOption(opt);
+
+ opt = new Option("b", "brokerAddress", true, "which address of the broker wants to be master");
Review Comment:
> which address of the broker wants to be master
to:
> The address of the broker which requires to become master
##########
tools/src/main/java/org/apache/rocketmq/tools/command/controller/ReElectMasterSubCommand.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.tools.command.controller;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.rocketmq.common.protocol.body.BrokerMemberGroup;
+import org.apache.rocketmq.common.protocol.header.namesrv.controller.ElectMasterResponseHeader;
+import org.apache.rocketmq.remoting.RPCHook;
+import org.apache.rocketmq.tools.admin.DefaultMQAdminExt;
+import org.apache.rocketmq.tools.command.SubCommand;
+import org.apache.rocketmq.tools.command.SubCommandException;
+
+public class ReElectMasterSubCommand implements SubCommand {
+
+ @Override
+ public String commandName() {
+ return "electMaster";
+ }
+
+ @Override
+ public String commandDesc() {
+ return "Re-elect the specified broker as master";
+ }
+
+ @Override
+ public Options buildCommandlineOptions(Options options) {
+ Option opt = new Option("a", "controllerAddress", true, "The address of controller");
+ opt.setRequired(true);
+ options.addOption(opt);
+
+ opt = new Option("b", "brokerAddress", true, "which address of the broker wants to be master");
+ opt.setRequired(true);
+ options.addOption(opt);
+
+ opt = new Option("n", "brokerName", true, "The broker name of the replicas want to operate");
Review Comment:
> The broker name of the replicas want to operate
to:
> The broker name of the replicas that require to be manipulated
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/manager/ReplicasInfoManager.java:
##########
@@ -160,25 +162,35 @@ public ControllerResult<ElectMasterResponseHeader> electMaster(
// First, check whether the master is still active
final String oldMaster = syncStateInfo.getMasterAddress();
if (StringUtils.isNoneEmpty(oldMaster) && brokerAlivePredicate.test(brokerInfo.getClusterName(), oldMaster)) {
- String err = String.format("The old master %s is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
- log.warn("{}", err);
- result.setCodeAndRemark(ResponseCode.CONTROLLER_INVALID_REQUEST, err);
- return result;
+
+ if (StringUtils.isBlank(assignBrokerAddress)) {
+ String err = String.format("The old master %s is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
+ log.warn("{}", err);
+ result.setCodeAndRemark(ResponseCode.CONTROLLER_INVALID_REQUEST, err);
+ return result;
+ }
+
+ if (StringUtils.equals(oldMaster, assignBrokerAddress)) {
+ String err = String.format("The Re-elect master the same as old master %s and old master is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
Review Comment:
> The Re-elect master the same as old master %s and old master is still alive, not need to elect new master for broker %s
to:
> The Re-elect master is the same as the old master %s which is still alive, no need to elect new master for broker %s
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/manager/ReplicasInfoManager.java:
##########
@@ -160,25 +161,30 @@ public ControllerResult<ElectMasterResponseHeader> electMaster(
// First, check whether the master is still active
final String oldMaster = syncStateInfo.getMasterAddress();
if (StringUtils.isNoneEmpty(oldMaster) && brokerAlivePredicate.test(brokerInfo.getClusterName(), oldMaster)) {
- String err = String.format("The old master %s is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
- log.warn("{}", err);
- result.setCodeAndRemark(ResponseCode.CONTROLLER_INVALID_REQUEST, err);
- return result;
+
+ if (StringUtils.isBlank(assignBrokerAddress) || StringUtils.equals(oldMaster, assignBrokerAddress)) {
+ String err = String.format("The old master %s is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
Review Comment:
> The old master %s is still alive, not need to elect new master for broker %s
to:
> The old master %s is still alive, no need to elect new master for broker %s
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] hzh0425 commented on a diff in pull request #4798: [ISSUE #4791] Add elect master command for admin CLI
Posted by GitBox <gi...@apache.org>.
hzh0425 commented on code in PR #4798:
URL: https://github.com/apache/rocketmq/pull/4798#discussion_r941323588
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/manager/ReplicasInfoManager.java:
##########
@@ -160,25 +161,30 @@ public ControllerResult<ElectMasterResponseHeader> electMaster(
// First, check whether the master is still active
final String oldMaster = syncStateInfo.getMasterAddress();
if (StringUtils.isNoneEmpty(oldMaster) && brokerAlivePredicate.test(brokerInfo.getClusterName(), oldMaster)) {
- String err = String.format("The old master %s is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
- log.warn("{}", err);
- result.setCodeAndRemark(ResponseCode.CONTROLLER_INVALID_REQUEST, err);
- return result;
+
+ if (StringUtils.isBlank(assignBrokerAddress) || StringUtils.equals(oldMaster, assignBrokerAddress)) {
+ String err = String.format("The old master %s is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
Review Comment:
This err log is inappropriate here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] coveralls commented on pull request #4798: [ISSUE #4791] Add elect master command for admin CLI
Posted by GitBox <gi...@apache.org>.
coveralls commented on PR #4798:
URL: https://github.com/apache/rocketmq/pull/4798#issuecomment-1208484986
[](https://coveralls.io/builds/51502057)
Coverage decreased (-0.1%) to 49.029% when pulling **89782d20c9ad584e70b2aab15bdd3986992388dc on mxsm:rocketmq-4791** into **ce274265db8af73e28624ecc1fc98cc984658844 on apache:develop**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] mxsm commented on a diff in pull request #4798: [ISSUE #4791] Add elect master command for admin CLI
Posted by GitBox <gi...@apache.org>.
mxsm commented on code in PR #4798:
URL: https://github.com/apache/rocketmq/pull/4798#discussion_r941482729
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/manager/ReplicasInfoManager.java:
##########
@@ -160,25 +161,30 @@ public ControllerResult<ElectMasterResponseHeader> electMaster(
// First, check whether the master is still active
final String oldMaster = syncStateInfo.getMasterAddress();
if (StringUtils.isNoneEmpty(oldMaster) && brokerAlivePredicate.test(brokerInfo.getClusterName(), oldMaster)) {
- String err = String.format("The old master %s is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
- log.warn("{}", err);
- result.setCodeAndRemark(ResponseCode.CONTROLLER_INVALID_REQUEST, err);
- return result;
+
+ if (StringUtils.isBlank(assignBrokerAddress) || StringUtils.equals(oldMaster, assignBrokerAddress)) {
+ String err = String.format("The old master %s is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
+ log.warn("{}", err);
+ result.setCodeAndRemark(ResponseCode.CONTROLLER_INVALID_REQUEST, err);
+ return result;
+ }
}
// Try elect a master in syncStateSet
if (syncStateSet.size() > 1) {
- boolean electSuccess = tryElectMaster(result, brokerName, syncStateSet, candidate ->
- !candidate.equals(syncStateInfo.getMasterAddress()) && brokerAlivePredicate.test(brokerInfo.getClusterName(), candidate));
+ boolean electSuccess = tryElectMaster(result, brokerName, assignBrokerAddress, syncStateSet, candidate ->
+ (StringUtils.isEmpty(assignBrokerAddress) ? !StringUtils.equals(candidate, syncStateInfo.getMasterAddress()) : true)
Review Comment:
Redundant check, I remove it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] ShannonDing commented on a diff in pull request #4798: [ISSUE #4791] Add elect master command for admin CLI
Posted by GitBox <gi...@apache.org>.
ShannonDing commented on code in PR #4798:
URL: https://github.com/apache/rocketmq/pull/4798#discussion_r941334171
##########
tools/src/main/java/org/apache/rocketmq/tools/command/controller/ElectMasterSubCommand.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.tools.command.controller;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.rocketmq.common.protocol.body.BrokerMemberGroup;
+import org.apache.rocketmq.common.protocol.header.namesrv.controller.ElectMasterResponseHeader;
+import org.apache.rocketmq.remoting.RPCHook;
+import org.apache.rocketmq.tools.admin.DefaultMQAdminExt;
+import org.apache.rocketmq.tools.command.SubCommand;
+import org.apache.rocketmq.tools.command.SubCommandException;
+
+public class ElectMasterSubCommand implements SubCommand {
+
+ @Override
+ public String commandName() {
+ return "electMaster";
+ }
+
+ @Override
+ public String commandDesc() {
+ return "Elects the specified broker as master";
+ }
+
+ @Override
+ public Options buildCommandlineOptions(Options options) {
+ Option opt = new Option("a", "controllerAddress", true, "the address of controller");
+ opt.setRequired(true);
+ options.addOption(opt);
+
+ opt = new Option("b", "brokerAddress", true, "the address of broker");
Review Comment:
which
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] codecov-commenter commented on pull request #4798: [ISSUE #4791] Add elect master command for admin CLI
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #4798:
URL: https://github.com/apache/rocketmq/pull/4798#issuecomment-1208485656
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4798?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#4798](https://codecov.io/gh/apache/rocketmq/pull/4798?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (89782d2) into [develop](https://codecov.io/gh/apache/rocketmq/commit/ce274265db8af73e28624ecc1fc98cc984658844?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ce27426) will **increase** coverage by `0.11%`.
> The diff coverage is `22.22%`.
```diff
@@ Coverage Diff @@
## develop #4798 +/- ##
=============================================
+ Coverage 44.86% 44.97% +0.11%
- Complexity 7589 7630 +41
=============================================
Files 977 978 +1
Lines 67957 68045 +88
Branches 8983 8996 +13
=============================================
+ Hits 30489 30604 +115
+ Misses 33673 33654 -19
+ Partials 3795 3787 -8
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/4798?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...g/apache/rocketmq/client/impl/MQClientAPIImpl.java](https://codecov.io/gh/apache/rocketmq/pull/4798/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9NUUNsaWVudEFQSUltcGwuamF2YQ==) | `23.38% <0.00%> (-0.28%)` | :arrow_down: |
| [...r/namesrv/controller/ElectMasterRequestHeader.java](https://codecov.io/gh/apache/rocketmq/pull/4798/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvaGVhZGVyL25hbWVzcnYvY29udHJvbGxlci9FbGVjdE1hc3RlclJlcXVlc3RIZWFkZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ntroller/processor/ControllerRequestProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/4798/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvY29udHJvbGxlci9wcm9jZXNzb3IvQ29udHJvbGxlclJlcXVlc3RQcm9jZXNzb3IuamF2YQ==) | `25.00% <0.00%> (-1.20%)` | :arrow_down: |
| [...apache/rocketmq/tools/admin/DefaultMQAdminExt.java](https://codecov.io/gh/apache/rocketmq/pull/4798/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2FkbWluL0RlZmF1bHRNUUFkbWluRXh0LmphdmE=) | `31.97% <0.00%> (-0.19%)` | :arrow_down: |
| [...he/rocketmq/tools/admin/DefaultMQAdminExtImpl.java](https://codecov.io/gh/apache/rocketmq/pull/4798/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2FkbWluL0RlZmF1bHRNUUFkbWluRXh0SW1wbC5qYXZh) | `25.67% <0.00%> (-0.03%)` | :arrow_down: |
| [.../apache/rocketmq/tools/command/MQAdminStartup.java](https://codecov.io/gh/apache/rocketmq/pull/4798/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2NvbW1hbmQvTVFBZG1pblN0YXJ0dXAuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ools/command/controller/ElectMasterSubCommand.java](https://codecov.io/gh/apache/rocketmq/pull/4798/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2NvbW1hbmQvY29udHJvbGxlci9FbGVjdE1hc3RlclN1YkNvbW1hbmQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...q/controller/impl/manager/ReplicasInfoManager.java](https://codecov.io/gh/apache/rocketmq/pull/4798/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvY29udHJvbGxlci9pbXBsL21hbmFnZXIvUmVwbGljYXNJbmZvTWFuYWdlci5qYXZh) | `66.39% <81.48%> (+5.94%)` | :arrow_up: |
| [...he/rocketmq/client/trace/AsyncTraceDispatcher.java](https://codecov.io/gh/apache/rocketmq/pull/4798/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvdHJhY2UvQXN5bmNUcmFjZURpc3BhdGNoZXIuamF2YQ==) | `81.42% <0.00%> (-1.43%)` | :arrow_down: |
| [...mq/store/ha/autoswitch/AutoSwitchHAConnection.java](https://codecov.io/gh/apache/rocketmq/pull/4798/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2hhL2F1dG9zd2l0Y2gvQXV0b1N3aXRjaEhBQ29ubmVjdGlvbi5qYXZh) | `74.38% <0.00%> (-0.82%)` | :arrow_down: |
| ... and [26 more](https://codecov.io/gh/apache/rocketmq/pull/4798/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] RongtongJin commented on a diff in pull request #4798: [ISSUE #4791] Add elect master command for admin CLI
Posted by GitBox <gi...@apache.org>.
RongtongJin commented on code in PR #4798:
URL: https://github.com/apache/rocketmq/pull/4798#discussion_r940811955
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/manager/ReplicasInfoManager.java:
##########
@@ -160,25 +161,30 @@ public ControllerResult<ElectMasterResponseHeader> electMaster(
// First, check whether the master is still active
final String oldMaster = syncStateInfo.getMasterAddress();
if (StringUtils.isNoneEmpty(oldMaster) && brokerAlivePredicate.test(brokerInfo.getClusterName(), oldMaster)) {
- String err = String.format("The old master %s is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
- log.warn("{}", err);
- result.setCodeAndRemark(ResponseCode.CONTROLLER_INVALID_REQUEST, err);
- return result;
+
+ if (StringUtils.isBlank(assignBrokerAddress) || StringUtils.equals(oldMaster, assignBrokerAddress)) {
Review Comment:
It would be better to combine the two if to the one
##########
tools/src/main/java/org/apache/rocketmq/tools/command/controller/ElectMasterSubCommand.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.tools.command.controller;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.rocketmq.common.protocol.body.BrokerMemberGroup;
+import org.apache.rocketmq.common.protocol.header.namesrv.controller.ElectMasterResponseHeader;
+import org.apache.rocketmq.remoting.RPCHook;
+import org.apache.rocketmq.tools.admin.DefaultMQAdminExt;
+import org.apache.rocketmq.tools.command.SubCommand;
+import org.apache.rocketmq.tools.command.SubCommandException;
+
+public class ElectMasterSubCommand implements SubCommand {
+
+ @Override
+ public String commandName() {
+ return "electMaster";
+ }
+
+ @Override
+ public String commandDesc() {
+ return "Elects the specified broker as master";
+ }
+
+ @Override
+ public Options buildCommandlineOptions(Options options) {
+ Option opt = new Option("a", "controllerAddress", true, "the address of controller");
+ opt.setRequired(true);
+ options.addOption(opt);
+
+ opt = new Option("b", "brokerAddress", true, "the address of broker");
+ opt.setRequired(true);
+ options.addOption(opt);
+
+ opt = new Option("n", "brokerName", true, "which broker to elect master");
Review Comment:
“The broker name of the replicas want to operate”
##########
tools/src/main/java/org/apache/rocketmq/tools/command/controller/ElectMasterSubCommand.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.tools.command.controller;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.rocketmq.common.protocol.body.BrokerMemberGroup;
+import org.apache.rocketmq.common.protocol.header.namesrv.controller.ElectMasterResponseHeader;
+import org.apache.rocketmq.remoting.RPCHook;
+import org.apache.rocketmq.tools.admin.DefaultMQAdminExt;
+import org.apache.rocketmq.tools.command.SubCommand;
+import org.apache.rocketmq.tools.command.SubCommandException;
+
+public class ElectMasterSubCommand implements SubCommand {
+
+ @Override
+ public String commandName() {
+ return "electMaster";
+ }
+
+ @Override
+ public String commandDesc() {
+ return "Elects the specified broker as master";
Review Comment:
"Elect the specified broker as master"
##########
common/src/main/java/org/apache/rocketmq/common/protocol/header/namesrv/controller/ElectMasterRequestHeader.java:
##########
@@ -20,15 +20,26 @@
import org.apache.rocketmq.remoting.exception.RemotingCommandException;
public class ElectMasterRequestHeader implements CommandCustomHeader {
+
+ private String clusterName;
+
private String brokerName;
+ private String brokerAddress;
Review Comment:
It would be better to add @CFNullable annotation
##########
tools/src/main/java/org/apache/rocketmq/tools/command/controller/ElectMasterSubCommand.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.tools.command.controller;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.rocketmq.common.protocol.body.BrokerMemberGroup;
+import org.apache.rocketmq.common.protocol.header.namesrv.controller.ElectMasterResponseHeader;
+import org.apache.rocketmq.remoting.RPCHook;
+import org.apache.rocketmq.tools.admin.DefaultMQAdminExt;
+import org.apache.rocketmq.tools.command.SubCommand;
+import org.apache.rocketmq.tools.command.SubCommandException;
+
+public class ElectMasterSubCommand implements SubCommand {
Review Comment:
How about rename to “ReElectMasterSubCommand”?
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/manager/ReplicasInfoManager.java:
##########
@@ -160,25 +161,30 @@ public ControllerResult<ElectMasterResponseHeader> electMaster(
// First, check whether the master is still active
final String oldMaster = syncStateInfo.getMasterAddress();
if (StringUtils.isNoneEmpty(oldMaster) && brokerAlivePredicate.test(brokerInfo.getClusterName(), oldMaster)) {
- String err = String.format("The old master %s is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
- log.warn("{}", err);
- result.setCodeAndRemark(ResponseCode.CONTROLLER_INVALID_REQUEST, err);
- return result;
+
+ if (StringUtils.isBlank(assignBrokerAddress) || StringUtils.equals(oldMaster, assignBrokerAddress)) {
+ String err = String.format("The old master %s is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
+ log.warn("{}", err);
+ result.setCodeAndRemark(ResponseCode.CONTROLLER_INVALID_REQUEST, err);
+ return result;
+ }
}
// Try elect a master in syncStateSet
if (syncStateSet.size() > 1) {
- boolean electSuccess = tryElectMaster(result, brokerName, syncStateSet, candidate ->
- !candidate.equals(syncStateInfo.getMasterAddress()) && brokerAlivePredicate.test(brokerInfo.getClusterName(), candidate));
+ boolean electSuccess = tryElectMaster(result, brokerName, assignBrokerAddress, syncStateSet, candidate ->
+ (StringUtils.isEmpty(assignBrokerAddress) ? !StringUtils.equals(candidate, syncStateInfo.getMasterAddress()) : true)
Review Comment:
This judgment seems unnecessary. -> StringUtils.isEmpty(assignBrokerAddress)
##########
tools/src/main/java/org/apache/rocketmq/tools/command/controller/ElectMasterSubCommand.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.tools.command.controller;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.rocketmq.common.protocol.body.BrokerMemberGroup;
+import org.apache.rocketmq.common.protocol.header.namesrv.controller.ElectMasterResponseHeader;
+import org.apache.rocketmq.remoting.RPCHook;
+import org.apache.rocketmq.tools.admin.DefaultMQAdminExt;
+import org.apache.rocketmq.tools.command.SubCommand;
+import org.apache.rocketmq.tools.command.SubCommandException;
+
+public class ElectMasterSubCommand implements SubCommand {
+
+ @Override
+ public String commandName() {
+ return "electMaster";
+ }
+
+ @Override
+ public String commandDesc() {
+ return "Elects the specified broker as master";
+ }
+
+ @Override
+ public Options buildCommandlineOptions(Options options) {
+ Option opt = new Option("a", "controllerAddress", true, "the address of controller");
+ opt.setRequired(true);
+ options.addOption(opt);
+
+ opt = new Option("b", "brokerAddress", true, "the address of broker");
Review Comment:
“the address of the broker wants to be master”
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] mxsm commented on a diff in pull request #4798: [ISSUE #4791] Add elect master command for admin CLI
Posted by GitBox <gi...@apache.org>.
mxsm commented on code in PR #4798:
URL: https://github.com/apache/rocketmq/pull/4798#discussion_r941483794
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/manager/ReplicasInfoManager.java:
##########
@@ -160,25 +161,30 @@ public ControllerResult<ElectMasterResponseHeader> electMaster(
// First, check whether the master is still active
final String oldMaster = syncStateInfo.getMasterAddress();
if (StringUtils.isNoneEmpty(oldMaster) && brokerAlivePredicate.test(brokerInfo.getClusterName(), oldMaster)) {
- String err = String.format("The old master %s is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
- log.warn("{}", err);
- result.setCodeAndRemark(ResponseCode.CONTROLLER_INVALID_REQUEST, err);
- return result;
+
+ if (StringUtils.isBlank(assignBrokerAddress) || StringUtils.equals(oldMaster, assignBrokerAddress)) {
+ String err = String.format("The old master %s is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
Review Comment:
I add a new err log for this case
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] RongtongJin merged pull request #4798: [ISSUE #4791] Add elect master command for admin CLI
Posted by GitBox <gi...@apache.org>.
RongtongJin merged PR #4798:
URL: https://github.com/apache/rocketmq/pull/4798
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] hzh0425 commented on pull request #4798: [ISSUE #4791] Add elect master command for admin CLI
Posted by GitBox <gi...@apache.org>.
hzh0425 commented on PR #4798:
URL: https://github.com/apache/rocketmq/pull/4798#issuecomment-1209375806
Thank you for your contribution. After modifying the above review suggestions, can you test your cli command and add a picture to prove the correctness? (The elect master command can correctly trigger the master election) @mxsm
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] mxsm commented on pull request #4798: [ISSUE #4791] Add elect master command for admin CLI
Posted by GitBox <gi...@apache.org>.
mxsm commented on PR #4798:
URL: https://github.com/apache/rocketmq/pull/4798#issuecomment-1209430711
> Thank you for your contribution. After modifying the above review suggestions, can you test your cli command and add a picture to prove the correctness? (The elect master command can correctly trigger the master election) @mxsm
OK, I will
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] hzh0425 commented on a diff in pull request #4798: [ISSUE #4791] Add elect master command for admin CLI
Posted by GitBox <gi...@apache.org>.
hzh0425 commented on code in PR #4798:
URL: https://github.com/apache/rocketmq/pull/4798#discussion_r941330669
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/manager/ReplicasInfoManager.java:
##########
@@ -160,25 +161,30 @@ public ControllerResult<ElectMasterResponseHeader> electMaster(
// First, check whether the master is still active
final String oldMaster = syncStateInfo.getMasterAddress();
if (StringUtils.isNoneEmpty(oldMaster) && brokerAlivePredicate.test(brokerInfo.getClusterName(), oldMaster)) {
- String err = String.format("The old master %s is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
- log.warn("{}", err);
- result.setCodeAndRemark(ResponseCode.CONTROLLER_INVALID_REQUEST, err);
- return result;
+
+ if (StringUtils.isBlank(assignBrokerAddress) || StringUtils.equals(oldMaster, assignBrokerAddress)) {
+ String err = String.format("The old master %s is still alive, not need to elect new master for broker %s", oldMaster, brokerInfo.getBrokerName());
+ log.warn("{}", err);
+ result.setCodeAndRemark(ResponseCode.CONTROLLER_INVALID_REQUEST, err);
+ return result;
+ }
}
// Try elect a master in syncStateSet
if (syncStateSet.size() > 1) {
- boolean electSuccess = tryElectMaster(result, brokerName, syncStateSet, candidate ->
- !candidate.equals(syncStateInfo.getMasterAddress()) && brokerAlivePredicate.test(brokerInfo.getClusterName(), candidate));
+ boolean electSuccess = tryElectMaster(result, brokerName, assignBrokerAddress, syncStateSet, candidate ->
+ (StringUtils.isEmpty(assignBrokerAddress) ? !StringUtils.equals(candidate, syncStateInfo.getMasterAddress()) : true)
Review Comment:
sorry, I don't understant, what's the meaning of this line?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] mxsm commented on pull request #4798: [ISSUE #4791] Add elect master command for admin CLI
Posted by GitBox <gi...@apache.org>.
mxsm commented on PR #4798:
URL: https://github.com/apache/rocketmq/pull/4798#issuecomment-1209526696

Correctness result proof。
environment:
two brokers are deployed in the WSL, Namesrv(Controller) is deployed in windows.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] RongtongJin commented on a diff in pull request #4798: [ISSUE #4791] Add elect master command for admin CLI
Posted by GitBox <gi...@apache.org>.
RongtongJin commented on code in PR #4798:
URL: https://github.com/apache/rocketmq/pull/4798#discussion_r940816961
##########
tools/src/main/java/org/apache/rocketmq/tools/command/controller/ElectMasterSubCommand.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.tools.command.controller;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.rocketmq.common.protocol.body.BrokerMemberGroup;
+import org.apache.rocketmq.common.protocol.header.namesrv.controller.ElectMasterResponseHeader;
+import org.apache.rocketmq.remoting.RPCHook;
+import org.apache.rocketmq.tools.admin.DefaultMQAdminExt;
+import org.apache.rocketmq.tools.command.SubCommand;
+import org.apache.rocketmq.tools.command.SubCommandException;
+
+public class ElectMasterSubCommand implements SubCommand {
+
+ @Override
+ public String commandName() {
+ return "electMaster";
+ }
+
+ @Override
+ public String commandDesc() {
+ return "Elects the specified broker as master";
Review Comment:
"Re elect the specified broker as master"
##########
tools/src/main/java/org/apache/rocketmq/tools/command/controller/ElectMasterSubCommand.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.rocketmq.tools.command.controller;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.rocketmq.common.protocol.body.BrokerMemberGroup;
+import org.apache.rocketmq.common.protocol.header.namesrv.controller.ElectMasterResponseHeader;
+import org.apache.rocketmq.remoting.RPCHook;
+import org.apache.rocketmq.tools.admin.DefaultMQAdminExt;
+import org.apache.rocketmq.tools.command.SubCommand;
+import org.apache.rocketmq.tools.command.SubCommandException;
+
+public class ElectMasterSubCommand implements SubCommand {
+
+ @Override
+ public String commandName() {
+ return "electMaster";
+ }
+
+ @Override
+ public String commandDesc() {
+ return "Elects the specified broker as master";
Review Comment:
"Re-elect the specified broker as master"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org