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/10 17:35:12 UTC
[GitHub] [rocketmq] TheR1sing3un opened a new pull request, #4809: feat(controller): add elect policy
TheR1sing3un opened a new pull request, #4809:
URL: https://github.com/apache/rocketmq/pull/4809
1. add epoch and maxOffset in heartbeat.
2. refactor elect logic, now we
elect a new master by elect policy(can expand).
3. add some unit tests
**Make sure set the target branch to `develop`**
## What is the purpose of the change
issue link: https://github.com/apache/rocketmq/pull/4484#issuecomment-1187616255
fix point 3, we choose to elect a master based on more information, such as epoch and maxOffset
## 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](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] TheR1sing3un commented on a diff in pull request #4809: [ISSUE #4813] Add elect policy for controller
Posted by GitBox <gi...@apache.org>.
TheR1sing3un commented on code in PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#discussion_r945294595
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/manager/ReplicasInfoManager.java:
##########
@@ -150,52 +153,42 @@ public ControllerResult<AlterSyncStateSetResponseHeader> alterSyncStateSet(
return result;
}
- public ControllerResult<ElectMasterResponseHeader> electMaster(
- final ElectMasterRequestHeader request, final BiPredicate<String, String> brokerAlivePredicate) {
+ public ControllerResult<ElectMasterResponseHeader> electMaster(final ElectMasterRequestHeader request, final ElectPolicy electPolicy) {
final String brokerName = request.getBrokerName();
final String assignBrokerAddress = request.getBrokerAddress();
final ControllerResult<ElectMasterResponseHeader> result = new ControllerResult<>(new ElectMasterResponseHeader());
if (isContainsBroker(brokerName)) {
final SyncStateInfo syncStateInfo = this.syncStateSetInfoTable.get(brokerName);
final BrokerInfo brokerInfo = this.replicaInfoTable.get(brokerName);
final Set<String> syncStateSet = syncStateInfo.getSyncStateSet();
- // First, check whether the master is still active
final String oldMaster = syncStateInfo.getMasterAddress();
- if (StringUtils.isNoneEmpty(oldMaster) && brokerAlivePredicate.test(brokerInfo.getClusterName(), oldMaster)) {
+ Set<String> allReplicaBrokers = controllerConfig.isEnableElectUncleanMaster() ? brokerInfo.getAllBroker() : null;
- if (StringUtils.isBlank(assignBrokerAddress)) {
- String err = String.format("The old master %s is still alive, no 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 is the same as the old master %s which is still alive, no 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, assignBrokerAddress, syncStateSet, candidate ->
- !candidate.equals(syncStateInfo.getMasterAddress()) && brokerAlivePredicate.test(brokerInfo.getClusterName(), candidate));
- if (electSuccess) {
- return result;
- }
+ // elect by policy
Review Comment:
done~
--
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 #4809: feat(controller): add elect policy
Posted by GitBox <gi...@apache.org>.
hzh0425 commented on PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#issuecomment-1212064464
wow, cool
--
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 #4809: [ISSUE #4813] Add elect policy for controller
Posted by GitBox <gi...@apache.org>.
RongtongJin commented on code in PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#discussion_r945177624
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/manager/ReplicasInfoManager.java:
##########
@@ -150,52 +153,42 @@ public ControllerResult<AlterSyncStateSetResponseHeader> alterSyncStateSet(
return result;
}
- public ControllerResult<ElectMasterResponseHeader> electMaster(
- final ElectMasterRequestHeader request, final BiPredicate<String, String> brokerAlivePredicate) {
+ public ControllerResult<ElectMasterResponseHeader> electMaster(final ElectMasterRequestHeader request, final ElectPolicy electPolicy) {
final String brokerName = request.getBrokerName();
final String assignBrokerAddress = request.getBrokerAddress();
final ControllerResult<ElectMasterResponseHeader> result = new ControllerResult<>(new ElectMasterResponseHeader());
if (isContainsBroker(brokerName)) {
final SyncStateInfo syncStateInfo = this.syncStateSetInfoTable.get(brokerName);
final BrokerInfo brokerInfo = this.replicaInfoTable.get(brokerName);
final Set<String> syncStateSet = syncStateInfo.getSyncStateSet();
- // First, check whether the master is still active
final String oldMaster = syncStateInfo.getMasterAddress();
- if (StringUtils.isNoneEmpty(oldMaster) && brokerAlivePredicate.test(brokerInfo.getClusterName(), oldMaster)) {
+ Set<String> allReplicaBrokers = controllerConfig.isEnableElectUncleanMaster() ? brokerInfo.getAllBroker() : null;
- if (StringUtils.isBlank(assignBrokerAddress)) {
- String err = String.format("The old master %s is still alive, no 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 is the same as the old master %s which is still alive, no 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, assignBrokerAddress, syncStateSet, candidate ->
- !candidate.equals(syncStateInfo.getMasterAddress()) && brokerAlivePredicate.test(brokerInfo.getClusterName(), candidate));
- if (electSuccess) {
- return result;
- }
+ // elect by policy
Review Comment:
Discarding the process of assginBrokerAddress will cause the function of [ReElectMasterSubCommand](https://github.com/apache/rocketmq/pull/4798) to fail
--
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] TheR1sing3un commented on a diff in pull request #4809: [ISSUE #4813] Add elect policy for controller
Posted by GitBox <gi...@apache.org>.
TheR1sing3un commented on code in PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#discussion_r944714268
##########
common/src/main/java/org/apache/rocketmq/common/protocol/header/namesrv/BrokerHeartbeatRequestHeader.java:
##########
@@ -28,7 +28,10 @@ public class BrokerHeartbeatRequestHeader implements CommandCustomHeader {
private String brokerAddr;
@CFNotNull
private String brokerName;
-
+ @CFNotNull
+ private int epoch;
+ @CFNotNull
+ private long maxOffset;
Review Comment:
Yep~ Done!
--
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] TheR1sing3un commented on a diff in pull request #4809: [ISSUE #4813] Add elect policy for controller
Posted by GitBox <gi...@apache.org>.
TheR1sing3un commented on code in PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#discussion_r944713172
##########
controller/src/main/java/org/apache/rocketmq/controller/pojo/BrokerLiveInfo.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.controller.pojo;
Review Comment:
got it~
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/manager/ReplicasInfoManager.java:
##########
@@ -201,6 +205,7 @@ public ControllerResult<ElectMasterResponseHeader> electMaster(
* @param filter return true if the candidate is available
* @return true if elect success
*/
+ @Deprecated
Review Comment:
done~
--
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] TheR1sing3un commented on a diff in pull request #4809: [ISSUE #4813] Add elect policy for controller
Posted by GitBox <gi...@apache.org>.
TheR1sing3un commented on code in PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#discussion_r944714782
##########
common/src/main/java/org/apache/rocketmq/common/protocol/header/namesrv/BrokerHeartbeatRequestHeader.java:
##########
@@ -28,7 +28,10 @@ public class BrokerHeartbeatRequestHeader implements CommandCustomHeader {
private String brokerAddr;
@CFNotNull
private String brokerName;
-
+ @CFNotNull
+ private int epoch;
+ @CFNotNull
+ private long maxOffset;
Review Comment:
Got it! Thks
--
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 pull request #4809: [ISSUE #4813] Add elect policy for controller
Posted by GitBox <gi...@apache.org>.
RongtongJin commented on PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#issuecomment-1214496235
Failed tests:
DLedgerControllerTest.testEnableElectUnCleanMaster:253 Values should be different. Actual: 127.0.0.1:9000
--
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 #4809: [ISSUE #4813] Add elect policy for controller
Posted by GitBox <gi...@apache.org>.
coveralls commented on PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#issuecomment-1214611297
[](https://coveralls.io/builds/51652379)
Coverage decreased (-0.2%) to 49.037% when pulling **07100c22a1b9ccee674ccf6e137a10882ea5429f on TheR1sing3un:develop** into **8dd07c84697d3709f1f4bef5d78089b7d171b6cd 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] RongtongJin commented on pull request #4809: feat(controller): add elect policy
Posted by GitBox <gi...@apache.org>.
RongtongJin commented on PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#issuecomment-1211447744
Hi @TheR1sing3un It would be better to open another issue to record this pull request.
--
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] TheR1sing3un commented on a diff in pull request #4809: [ISSUE #4813] Add elect policy for controller
Posted by GitBox <gi...@apache.org>.
TheR1sing3un commented on code in PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#discussion_r944706336
##########
controller/src/main/java/org/apache/rocketmq/controller/elect/ElectPolicy.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.controller.elect;
+
+import org.apache.rocketmq.controller.pojo.BrokerLiveInfo;
+
+import java.util.Set;
+import java.util.function.BiFunction;
+import java.util.function.BiPredicate;
+
+public abstract class ElectPolicy {
Review Comment:
got it! thks~
##########
controller/src/main/java/org/apache/rocketmq/controller/elect/impl/DefaultElectPolicy.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.controller.elect.impl;
+
+import io.netty.util.internal.StringUtil;
Review Comment:
yep! thks
--
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 #4809: [ISSUE #4813] Add elect policy for controller
Posted by GitBox <gi...@apache.org>.
RongtongJin merged PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809
--
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 #4809: [ISSUE #4813] Add elect policy for controller
Posted by GitBox <gi...@apache.org>.
hzh0425 commented on PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#issuecomment-1213918411
Pls fix the misspell and ci
--
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] TheR1sing3un commented on pull request #4809: feat(controller): add elect policy
Posted by GitBox <gi...@apache.org>.
TheR1sing3un commented on PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#issuecomment-1211607713
> Is there an issue related? Fix the title following the template guideline
OK, I will open an issue to record this pr soon.
--
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] lizhanhui commented on pull request #4809: feat(controller): add elect policy
Posted by GitBox <gi...@apache.org>.
lizhanhui commented on PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#issuecomment-1211606118
Is there an issue related? Fix the title following the template guideline
--
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 #4809: feat(controller): add elect policy
Posted by GitBox <gi...@apache.org>.
RongtongJin commented on code in PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#discussion_r943027181
##########
common/src/main/java/org/apache/rocketmq/common/protocol/header/namesrv/BrokerHeartbeatRequestHeader.java:
##########
@@ -28,7 +28,10 @@ public class BrokerHeartbeatRequestHeader implements CommandCustomHeader {
private String brokerAddr;
@CFNotNull
private String brokerName;
-
+ @CFNotNull
+ private int epoch;
+ @CFNotNull
+ private long maxOffset;
Review Comment:
Since this heartbeat package will also be used by nameserver, in order to ensure compatibility, it is necessary to set the newly added field to **@CFNullable**
##########
controller/src/main/java/org/apache/rocketmq/controller/pojo/BrokerLiveInfo.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.controller.pojo;
Review Comment:
It would be better to follow the current style and not create a new pojo directory
##########
common/src/main/java/org/apache/rocketmq/common/protocol/header/namesrv/BrokerHeartbeatRequestHeader.java:
##########
@@ -28,7 +28,10 @@ public class BrokerHeartbeatRequestHeader implements CommandCustomHeader {
private String brokerAddr;
@CFNotNull
private String brokerName;
-
+ @CFNotNull
+ private int epoch;
+ @CFNotNull
+ private long maxOffset;
Review Comment:
IMO, It would be better to add confirmOffset to the heartbeat packet.
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/manager/ReplicasInfoManager.java:
##########
@@ -201,6 +205,7 @@ public ControllerResult<ElectMasterResponseHeader> electMaster(
* @param filter return true if the candidate is available
* @return true if elect success
*/
+ @Deprecated
Review Comment:
If this method is no longer used, it can be deleted. It has not been released yet.
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/DLedgerController.java:
##########
@@ -77,20 +81,25 @@ public class DLedgerController implements Controller {
private final DLedgerControllerStateMachine statemachine;
// Usr for checking whether the broker is alive
private BiPredicate<String, String> brokerAlivePredicate;
+
+ private BiFunction<String, String, BrokerLiveInfo> additionalInfoGetter;
+
+
private AtomicBoolean isScheduling = new AtomicBoolean(false);
public DLedgerController(final ControllerConfig config, final BiPredicate<String, String> brokerAlivePredicate) {
- this(config, brokerAlivePredicate, null, null, null);
+ this(config, brokerAlivePredicate, null, null, null, null);
}
public DLedgerController(final ControllerConfig controllerConfig,
final BiPredicate<String, String> brokerAlivePredicate, final NettyServerConfig nettyServerConfig,
- final NettyClientConfig nettyClientConfig, final ChannelEventListener channelEventListener) {
+ final NettyClientConfig nettyClientConfig, final ChannelEventListener channelEventListener,
+ BiFunction<String, String, BrokerLiveInfo> additionalInfoGetter) {
Review Comment:
It would better to use electPolicy to construct here instead of brokerAlivePredicate and additionalInfoGetter
--
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] TheR1sing3un commented on a diff in pull request #4809: [ISSUE #4813] Add elect policy for controller
Posted by GitBox <gi...@apache.org>.
TheR1sing3un commented on code in PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#discussion_r944706660
##########
common/src/main/java/org/apache/rocketmq/common/protocol/header/namesrv/BrokerHeartbeatRequestHeader.java:
##########
@@ -28,7 +28,10 @@ public class BrokerHeartbeatRequestHeader implements CommandCustomHeader {
private String brokerAddr;
@CFNotNull
private String brokerName;
-
+ @CFNotNull
+ private int epoch;
+ @CFNotNull
+ private long maxOffset;
Review Comment:
right! thks
--
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] TheR1sing3un commented on a diff in pull request #4809: [ISSUE #4813] Add elect policy for controller
Posted by GitBox <gi...@apache.org>.
TheR1sing3un commented on code in PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#discussion_r944712670
##########
controller/src/main/java/org/apache/rocketmq/controller/impl/DLedgerController.java:
##########
@@ -77,20 +81,25 @@ public class DLedgerController implements Controller {
private final DLedgerControllerStateMachine statemachine;
// Usr for checking whether the broker is alive
private BiPredicate<String, String> brokerAlivePredicate;
+
+ private BiFunction<String, String, BrokerLiveInfo> additionalInfoGetter;
+
+
private AtomicBoolean isScheduling = new AtomicBoolean(false);
public DLedgerController(final ControllerConfig config, final BiPredicate<String, String> brokerAlivePredicate) {
- this(config, brokerAlivePredicate, null, null, null);
+ this(config, brokerAlivePredicate, null, null, null, null);
}
public DLedgerController(final ControllerConfig controllerConfig,
final BiPredicate<String, String> brokerAlivePredicate, final NettyServerConfig nettyServerConfig,
- final NettyClientConfig nettyClientConfig, final ChannelEventListener channelEventListener) {
+ final NettyClientConfig nettyClientConfig, final ChannelEventListener channelEventListener,
+ BiFunction<String, String, BrokerLiveInfo> additionalInfoGetter) {
Review Comment:
Right! But brokerAlivePredicate still needs to be used! So I just replace the addtionalInfoGetter with electPolicy there. Thanks for your suggestion~
--
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 #4809: [ISSUE #4813] Add elect policy for controller
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#issuecomment-1214609394
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4809?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 [#4809](https://codecov.io/gh/apache/rocketmq/pull/4809?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (07100c2) into [develop](https://codecov.io/gh/apache/rocketmq/commit/8dd07c84697d3709f1f4bef5d78089b7d171b6cd?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8dd07c8) will **decrease** coverage by `0.19%`.
> The diff coverage is `54.41%`.
```diff
@@ Coverage Diff @@
## develop #4809 +/- ##
=============================================
- Coverage 44.94% 44.74% -0.20%
+ Complexity 7640 7631 -9
=============================================
Files 980 982 +2
Lines 68135 68222 +87
Branches 9014 9022 +8
=============================================
- Hits 30622 30526 -96
- Misses 33729 33901 +172
- Partials 3784 3795 +11
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/4809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...a/org/apache/rocketmq/broker/BrokerController.java](https://codecov.io/gh/apache/rocketmq/pull/4809/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvQnJva2VyQ29udHJvbGxlci5qYXZh) | `43.80% <0.00%> (-0.09%)` | :arrow_down: |
| [...l/header/namesrv/BrokerHeartbeatRequestHeader.java](https://codecov.io/gh/apache/rocketmq/pull/4809/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvaGVhZGVyL25hbWVzcnYvQnJva2VySGVhcnRiZWF0UmVxdWVzdEhlYWRlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...oller/RegisterBrokerToControllerRequestHeader.java](https://codecov.io/gh/apache/rocketmq/pull/4809/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvaGVhZGVyL25hbWVzcnYvY29udHJvbGxlci9SZWdpc3RlckJyb2tlclRvQ29udHJvbGxlclJlcXVlc3RIZWFkZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ketmq/store/ha/autoswitch/AutoSwitchHAService.java](https://codecov.io/gh/apache/rocketmq/pull/4809/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-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2hhL2F1dG9zd2l0Y2gvQXV0b1N3aXRjaEhBU2VydmljZS5qYXZh) | `56.06% <0.00%> (-0.29%)` | :arrow_down: |
| [...org/apache/rocketmq/broker/out/BrokerOuterAPI.java](https://codecov.io/gh/apache/rocketmq/pull/4809/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvb3V0L0Jyb2tlck91dGVyQVBJLmphdmE=) | `18.60% <16.66%> (-0.12%)` | :arrow_down: |
| [...controller/impl/DefaultBrokerHeartbeatManager.java](https://codecov.io/gh/apache/rocketmq/pull/4809/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-Y29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvY29udHJvbGxlci9pbXBsL0RlZmF1bHRCcm9rZXJIZWFydGJlYXRNYW5hZ2VyLmphdmE=) | `71.26% <44.44%> (-9.23%)` | :arrow_down: |
| [...org/apache/rocketmq/controller/BrokerLiveInfo.java](https://codecov.io/gh/apache/rocketmq/pull/4809/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-Y29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvY29udHJvbGxlci9Ccm9rZXJMaXZlSW5mby5qYXZh) | `53.65% <53.65%> (ø)` | |
| [...he/rocketmq/broker/controller/ReplicasManager.java](https://codecov.io/gh/apache/rocketmq/pull/4809/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvY29udHJvbGxlci9SZXBsaWNhc01hbmFnZXIuamF2YQ==) | `44.97% <60.00%> (+0.08%)` | :arrow_up: |
| [.../apache/rocketmq/controller/ControllerManager.java](https://codecov.io/gh/apache/rocketmq/pull/4809/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-Y29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvY29udHJvbGxlci9Db250cm9sbGVyTWFuYWdlci5qYXZh) | `75.00% <75.00%> (ø)` | |
| [...etmq/controller/elect/impl/DefaultElectPolicy.java](https://codecov.io/gh/apache/rocketmq/pull/4809/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-Y29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvY29udHJvbGxlci9lbGVjdC9pbXBsL0RlZmF1bHRFbGVjdFBvbGljeS5qYXZh) | `75.00% <75.00%> (ø)` | |
| ... and [34 more](https://codecov.io/gh/apache/rocketmq/pull/4809/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: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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] TheR1sing3un commented on pull request #4809: feat(controller): add elect policy
Posted by GitBox <gi...@apache.org>.
TheR1sing3un commented on PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#issuecomment-1211036357
@RongtongJin Hi~ Please check! Thks!🤣
--
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 #4809: feat(controller): add elect policy
Posted by GitBox <gi...@apache.org>.
mxsm commented on code in PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#discussion_r943250680
##########
controller/src/main/java/org/apache/rocketmq/controller/elect/ElectPolicy.java:
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.controller.elect;
+
+import org.apache.rocketmq.controller.pojo.BrokerLiveInfo;
+
+import java.util.Set;
+import java.util.function.BiFunction;
+import java.util.function.BiPredicate;
+
+public abstract class ElectPolicy {
Review Comment:
It would be better to define as interface and add a getElectPolicyType(), It is convenient to expand new election strategies in the future。
##########
broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java:
##########
@@ -1685,7 +1685,9 @@ protected void sendHeartbeat() {
this.brokerConfig.getBrokerName(),
this.brokerConfig.getBrokerId(),
this.brokerConfig.getSendHeartbeatTimeoutMillis(),
- this.brokerConfig.isInBrokerContainer()
+ this.brokerConfig.isInBrokerContainer(),
+ this.replicasManager.getLastEpoch(),
+ this.messageStore.getMaxPhyOffset()
Review Comment:
code formate
##########
common/src/main/java/org/apache/rocketmq/common/protocol/header/namesrv/BrokerHeartbeatRequestHeader.java:
##########
@@ -28,7 +28,10 @@ public class BrokerHeartbeatRequestHeader implements CommandCustomHeader {
private String brokerAddr;
@CFNotNull
private String brokerName;
-
+ @CFNotNull
+ private int epoch;
+ @CFNotNull
+ private long maxOffset;
Review Comment:
epoch and maxOffset if you want to add @CFNotNull annotation suggest change the type to Integer and Long
##########
controller/src/main/java/org/apache/rocketmq/controller/elect/impl/DefaultElectPolicy.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.controller.elect.impl;
+
+import io.netty.util.internal.StringUtil;
Review Comment:
Invalid imports need to be removed
--
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 #4809: [ISSUE #4813] Add elect policy for controller
Posted by GitBox <gi...@apache.org>.
coveralls commented on PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#issuecomment-1214611295
[](https://coveralls.io/builds/51652379)
Coverage decreased (-0.2%) to 49.037% when pulling **07100c22a1b9ccee674ccf6e137a10882ea5429f on TheR1sing3un:develop** into **8dd07c84697d3709f1f4bef5d78089b7d171b6cd 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] RongtongJin commented on pull request #4809: [ISSUE #4813] Add elect policy for controller
Posted by GitBox <gi...@apache.org>.
RongtongJin commented on PR #4809:
URL: https://github.com/apache/rocketmq/pull/4809#issuecomment-1214666046
@hzh0425 @mxsm Plz help review
--
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