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/01/11 10:17:49 UTC
[GitHub] [rocketmq] XiaoyiPeng opened a new pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
XiaoyiPeng opened a new pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746
**Make sure set the target branch to `develop`**
## What is the purpose of the change
For issue #3681
## Brief changelog
Add the command to check master-slave synchronization status.
## Verifying this change
--
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 #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#issuecomment-1085372135
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3746?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 [#3746](https://codecov.io/gh/apache/rocketmq/pull/3746?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6505cab) into [develop](https://codecov.io/gh/apache/rocketmq/commit/b606d9c638c2b9bf018d9a9968fdc9e5d6acf07f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b606d9c) will **increase** coverage by `0.34%`.
> The diff coverage is `38.70%`.
```diff
@@ Coverage Diff @@
## develop #3746 +/- ##
=============================================
+ Coverage 47.53% 47.87% +0.34%
- Complexity 4931 5009 +78
=============================================
Files 633 635 +2
Lines 42497 42622 +125
Branches 5579 5583 +4
=============================================
+ Hits 20201 20406 +205
+ Misses 19798 19710 -88
- Partials 2498 2506 +8
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3746?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/common/protocol/RequestCode.java](https://codecov.io/gh/apache/rocketmq/pull/3746/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvUmVxdWVzdENvZGUuamF2YQ==) | `0.00% <ø> (ø)` | |
| [...apache/rocketmq/tools/admin/DefaultMQAdminExt.java](https://codecov.io/gh/apache/rocketmq/pull/3746/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=) | `41.86% <ø> (+0.91%)` | :arrow_up: |
| [...he/rocketmq/tools/admin/DefaultMQAdminExtImpl.java](https://codecov.io/gh/apache/rocketmq/pull/3746/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) | `39.95% <0.00%> (+0.25%)` | :arrow_up: |
| [.../apache/rocketmq/tools/command/MQAdminStartup.java](https://codecov.io/gh/apache/rocketmq/pull/3746/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/broker/GetMasterSlaveDiffCommand.java](https://codecov.io/gh/apache/rocketmq/pull/3746/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-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2NvbW1hbmQvYnJva2VyL0dldE1hc3RlclNsYXZlRGlmZkNvbW1hbmQuamF2YQ==) | `34.24% <34.24%> (ø)` | |
| [...g/apache/rocketmq/client/impl/MQClientAPIImpl.java](https://codecov.io/gh/apache/rocketmq/pull/3746/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==) | `14.06% <37.50%> (+0.19%)` | :arrow_up: |
| [...ocketmq/broker/processor/AdminBrokerProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3746/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL0FkbWluQnJva2VyUHJvY2Vzc29yLmphdmE=) | `34.85% <88.88%> (+0.51%)` | :arrow_up: |
| [...va/org/apache/rocketmq/store/FlushDiskWatcher.java](https://codecov.io/gh/apache/rocketmq/pull/3746/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-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL0ZsdXNoRGlza1dhdGNoZXIuamF2YQ==) | `81.25% <0.00%> (-9.38%)` | :arrow_down: |
| [...in/java/org/apache/rocketmq/test/util/MQAdmin.java](https://codecov.io/gh/apache/rocketmq/pull/3746/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL01RQWRtaW4uamF2YQ==) | `38.88% <0.00%> (-5.56%)` | :arrow_down: |
| [...org/apache/rocketmq/store/ha/WaitNotifyObject.java](https://codecov.io/gh/apache/rocketmq/pull/3746/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-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2hhL1dhaXROb3RpZnlPYmplY3QuamF2YQ==) | `66.07% <0.00%> (-5.36%)` | :arrow_down: |
| ... and [49 more](https://codecov.io/gh/apache/rocketmq/pull/3746/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) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/3746?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3746?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b606d9c...6505cab](https://codecov.io/gh/apache/rocketmq/pull/3746?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] XiaoyiPeng commented on a change in pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
XiaoyiPeng commented on a change in pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#discussion_r812607286
##########
File path: store/src/test/java/org/apache/rocketmq/store/ha/WaitNotifyObjectTest.java
##########
@@ -40,4 +40,57 @@ public void run() {
Assert.assertEquals(0, waitNotifyObject.waitingThreadTable.size());
}
+ @Test
+ public void allWaitForRunning() throws Exception {
+
+ final int threadNum = 5;
+ final long waitIntervalMs = 100L;
+ final CountDownLatch latch = new CountDownLatch(threadNum);
+ final WaitNotifyObject waitNotifyObject = new WaitNotifyObject() {
+ @Override
+ protected void onWaitEnd() {
+ latch.countDown();
+ }
+ };
+ long start = System.nanoTime();
+ for (int i = 0; i < threadNum; i++) {
+ Thread t = new Thread(new Runnable() {
+ @Override
+ public void run() {
+ waitNotifyObject.allWaitForRunning(waitIntervalMs);
+ }
+ });
+ t.start();
+ t.join();
+ }
+
+ latch.await();
+ long elapsed = (System.nanoTime() - start) / 1000000;
+ Assert.assertEquals(threadNum, waitNotifyObject.waitingThreadTable.size());
+ Assert.assertTrue(elapsed >= threadNum * waitIntervalMs);
+ }
+
+ @Test
+ public void wakeup() throws Exception {
+ final long waitIntervalMs = 3000L;
+ final long sleepMs = 500L;
+ final WaitNotifyObject waitNotifyObject = new WaitNotifyObject();
+ long start = System.nanoTime();
+ Thread t = new Thread(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ Thread.currentThread().sleep(sleepMs);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ waitNotifyObject.wakeup();
+ }
+ });
+ t.start();
+ waitNotifyObject.waitForRunning(waitIntervalMs);
+ long elapsed = (System.nanoTime() - start) / 1000000;
+ Assert.assertTrue(elapsed >= sleepMs && elapsed < waitIntervalMs);
+ }
+
Review comment:
In fact, you are right.
The reason I add unit test for class `WaitNotifyObject` is because **coverage check** is always failed, and **coverage check** indicates the coverage decreased due to this class `WaitNotifyObject`.
--
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] XiaoyiPeng commented on pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
XiaoyiPeng commented on pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#issuecomment-1048467898
Hi @panzhi33 , @ni-ze , @Git-Yang ,
Could you help me to review this PR at your leisure? Thank you.
The related ISSUE #3681 is tagged with 4.9.1 milestone
--
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] XiaoyiPeng commented on a change in pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
XiaoyiPeng commented on a change in pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#discussion_r812607286
##########
File path: store/src/test/java/org/apache/rocketmq/store/ha/WaitNotifyObjectTest.java
##########
@@ -40,4 +40,57 @@ public void run() {
Assert.assertEquals(0, waitNotifyObject.waitingThreadTable.size());
}
+ @Test
+ public void allWaitForRunning() throws Exception {
+
+ final int threadNum = 5;
+ final long waitIntervalMs = 100L;
+ final CountDownLatch latch = new CountDownLatch(threadNum);
+ final WaitNotifyObject waitNotifyObject = new WaitNotifyObject() {
+ @Override
+ protected void onWaitEnd() {
+ latch.countDown();
+ }
+ };
+ long start = System.nanoTime();
+ for (int i = 0; i < threadNum; i++) {
+ Thread t = new Thread(new Runnable() {
+ @Override
+ public void run() {
+ waitNotifyObject.allWaitForRunning(waitIntervalMs);
+ }
+ });
+ t.start();
+ t.join();
+ }
+
+ latch.await();
+ long elapsed = (System.nanoTime() - start) / 1000000;
+ Assert.assertEquals(threadNum, waitNotifyObject.waitingThreadTable.size());
+ Assert.assertTrue(elapsed >= threadNum * waitIntervalMs);
+ }
+
+ @Test
+ public void wakeup() throws Exception {
+ final long waitIntervalMs = 3000L;
+ final long sleepMs = 500L;
+ final WaitNotifyObject waitNotifyObject = new WaitNotifyObject();
+ long start = System.nanoTime();
+ Thread t = new Thread(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ Thread.currentThread().sleep(sleepMs);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ waitNotifyObject.wakeup();
+ }
+ });
+ t.start();
+ waitNotifyObject.waitForRunning(waitIntervalMs);
+ long elapsed = (System.nanoTime() - start) / 1000000;
+ Assert.assertTrue(elapsed >= sleepMs && elapsed < waitIntervalMs);
+ }
+
Review comment:
In fact, you are right.
The reason I add unit test for class `WaitNotifyObject` is because **coverage check** is always failed, and the **coverage check** indicates the coverage decreased due to this class `WaitNotifyObject`.
--
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 edited a comment on pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#issuecomment-1009854969
[](https://coveralls.io/builds/47902274)
Coverage decreased (-0.02%) to 51.936% when pulling **6505cab4e92f20a9dd963733a8172b8df4b1d38a on XiaoyiPeng:issue#3681** into **e3b67484b0ea3172cf0555e39efe36c0030c53b3 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] coveralls edited a comment on pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#issuecomment-1009854969
[](https://coveralls.io/builds/46773560)
Coverage increased (+0.03%) to 51.58% when pulling **451fa16558444de4aa4da147559e01c7c9d35de1 on XiaoyiPeng:issue#3681** into **b606d9c638c2b9bf018d9a9968fdc9e5d6acf07f 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] coveralls edited a comment on pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#issuecomment-1009854969
[](https://coveralls.io/builds/46767733)
Coverage decreased (-0.007%) to 51.545% when pulling **c8d4733398fd93d476c0006b831d09ad972d1f6f on XiaoyiPeng:issue#3681** into **b606d9c638c2b9bf018d9a9968fdc9e5d6acf07f 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] XiaoyiPeng commented on a change in pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
XiaoyiPeng commented on a change in pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#discussion_r840191185
##########
File path: tools/src/test/java/org/apache/rocketmq/tools/command/broker/GetMasterSlaveDiffCommandTest.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.broker;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Options;
+import org.apache.commons.cli.PosixParser;
+import org.apache.rocketmq.client.exception.MQBrokerException;
+import org.apache.rocketmq.client.exception.MQClientException;
+import org.apache.rocketmq.common.protocol.body.ClusterInfo;
+import org.apache.rocketmq.common.protocol.route.BrokerData;
+import org.apache.rocketmq.remoting.exception.RemotingException;
+import org.apache.rocketmq.srvutil.ServerUtil;
+import org.apache.rocketmq.tools.admin.DefaultMQAdminExt;
+import org.apache.rocketmq.tools.command.SubCommandException;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.internal.util.collections.Sets;
+
+import java.lang.reflect.Field;
+import java.util.HashMap;
+import java.util.Set;
+
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
+
+public class GetMasterSlaveDiffCommandTest {
+
+ private GetMasterSlaveDiffCommand cmd;
+
+ private static DefaultMQAdminExt defaultMQAdminExt;
+
+ private static final String CLUSTER_NAME = "DefaultCluster";
+
+ private static final String BROKER_NAME = "broker-a";
+
+ @Before
+ public void before() throws NoSuchFieldException, IllegalAccessException, RemotingException, MQBrokerException,
+ InterruptedException, MQClientException {
+
+ ClusterInfo clusterInfo = new ClusterInfo();
+ HashMap<String, Set<String>> clusterMap = new HashMap<>(4);
+ clusterMap.put(CLUSTER_NAME, Sets.newSet(BROKER_NAME));
+ clusterInfo.setClusterAddrTable(clusterMap);
+
+ HashMap<Long, String> brokerMap = new HashMap<>(4);
+ brokerMap.put(0L, "127.0.0.1:10911");
+ BrokerData brokerData = new BrokerData(CLUSTER_NAME, BROKER_NAME, brokerMap);
+ HashMap<String, BrokerData> brokerAddrTable = new HashMap<>();
+ brokerAddrTable.put(BROKER_NAME, brokerData);
+ clusterInfo.setBrokerAddrTable(brokerAddrTable);
+
+ defaultMQAdminExt = new DefaultMQAdminExt();
+ defaultMQAdminExt = spy(defaultMQAdminExt);
+
+ cmd = new GetMasterSlaveDiffCommand();
+ Field field = cmd.getClass().getDeclaredField("defaultMQAdminExt");
+ field.setAccessible(true);
+ field.set(cmd, defaultMQAdminExt);
+
+ doReturn(clusterInfo).when(defaultMQAdminExt).examineBrokerClusterInfo();
+ doReturn(1024L).when(defaultMQAdminExt).getMasterSlaveDiff(anyString());
+ }
+
+ @After
+ public void after() {}
+
+ @Test
+ public void testExecute() throws SubCommandException, RemotingException, MQBrokerException, InterruptedException, MQClientException {
+ Options options = ServerUtil.buildCommandlineOptions(new Options());
+ String[] subargs = new String[] {"-b " + BROKER_NAME , "-c " + CLUSTER_NAME};
+ final CommandLine commandLine =
+ ServerUtil.parseCmdLine("mqadmin " + cmd.commandName(), subargs, cmd.buildCommandlineOptions(options), new PosixParser());
+ cmd.execute(commandLine, options, null);
Review comment:
Hi, @vongosling
Thanks for your review, in package `org.apache.rocketmq.tools.command.broker`, none of the classes has assert for cmd execute.
For consistency of code style, I didn't add assert before
But just now, I've added the assert for cmd execute, please review again. Thank you very much.
--
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] ni-ze commented on a change in pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
ni-ze commented on a change in pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#discussion_r782657839
##########
File path: tools/src/main/java/org/apache/rocketmq/tools/command/broker/GetMasterSlaveDiffCommand.java
##########
@@ -0,0 +1,151 @@
+/*
+ * 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.broker;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.rocketmq.common.MixAll;
+import org.apache.rocketmq.common.protocol.body.ClusterInfo;
+import org.apache.rocketmq.common.protocol.route.BrokerData;
+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;
+
+import java.util.HashMap;
+import java.util.Set;
+
+public class GetMasterSlaveDiffCommand implements SubCommand {
+ @Override
+ public String commandName() {
+ return "getMasterSlaveDiff";
+ }
+
+ @Override
+ public String commandDesc() {
+ return "Get number of bytes that slave falls behind master.";
+ }
+
+ @Override
+ public Options buildCommandlineOptions(final Options options) {
+ Option opt = new Option("b", "brokerName", true, "Broker Name");
+ opt.setRequired(false);
+ options.addOption(opt);
+
+ opt = new Option("c", "clusterName", true, "Cluster Name");
+ opt.setRequired(false);
+ options.addOption(opt);
+
+ return options;
+ }
+
+ @Override
+ public void execute(final CommandLine commandLine, final Options options,
+ final RPCHook rpcHook) throws SubCommandException {
+ DefaultMQAdminExt defaultMQAdminExt = new DefaultMQAdminExt(rpcHook);
+ defaultMQAdminExt.setInstanceName(Long.toString(System.currentTimeMillis()));
+ try {
+ if (commandLine.hasOption('b')) {
+ String brokerAddr = commandLine.getOptionValue('b').trim();
+ defaultMQAdminExt.start();
+ printDiffForBroker(defaultMQAdminExt, brokerAddr);
+ } else if (commandLine.hasOption('c')) {
+ String clusterName = commandLine.getOptionValue('c').trim();
+ defaultMQAdminExt.start();
+ printDiffForCluster(defaultMQAdminExt, clusterName);
+ }
+ } catch (Exception e) {
+ throw new SubCommandException(this.getClass().getSimpleName() + " command failed", e);
+ } finally {
+ defaultMQAdminExt.shutdown();
+ }
+ }
+
+ private void printDiffForCluster(final DefaultMQAdminExt defaultMQAdminExt, String clusterName) throws SubCommandException {
+ ClusterInfo clusterInfo = null;
+ try {
+ clusterInfo = defaultMQAdminExt.examineBrokerClusterInfo();
+ } catch (Exception e) {
+ throw new SubCommandException(this.getClass().getSimpleName() + " command failed", e);
+ }
+
+ HashMap<String, Set<String>> clusterMap = clusterInfo.getClusterAddrTable();
+ Set<String> brokerNameSet = clusterMap.get(clusterName);
+ if (brokerNameSet == null || brokerNameSet.isEmpty()) {
+ System.out.printf("Can not find brokers for the cluster named %s!%n", clusterName);
+ return;
+ } else {
+ System.out.printf("%-24s %-24s %-24s %-14s%n",
+ "#Cluster Name",
+ "#Broker Name",
+ "#Broker Address(Master)",
+ "#Diff(Bytes)"
+ );
+ for (String brokerName : brokerNameSet) {
+ HashMap<String/* brokerName */, BrokerData> brokerAddrTable = clusterInfo.getBrokerAddrTable();
+ BrokerData brokerData;
+ if ((brokerData = brokerAddrTable.get(brokerName)) == null) {
+ continue;
+ } else {
+ String masterBrokerAddr = brokerData.getBrokerAddrs().get(MixAll.MASTER_ID);
+ if (StringUtils.isBlank(masterBrokerAddr))
+ continue;
+ try {
Review comment:
suggest all code in loop moving into try catch block.
--
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 edited a comment on pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#issuecomment-1009854969
[](https://coveralls.io/builds/46763226)
Coverage decreased (-0.02%) to 51.536% when pulling **9875a68f2b31bcbd998a6ce32f2b354c6b04094e on XiaoyiPeng:issue#3681** into **b606d9c638c2b9bf018d9a9968fdc9e5d6acf07f 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] coveralls edited a comment on pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#issuecomment-1009854969
[](https://coveralls.io/builds/45553507)
Coverage decreased (-0.07%) to 53.125% when pulling **59f81836204a13e59d04bb42810ccc868b06afb0 on XiaoyiPeng:issue#3681** into **0b612ff813051cb3d4272bbd50ed280cbe4da244 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] panzhi33 commented on a change in pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
panzhi33 commented on a change in pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#discussion_r812600551
##########
File path: store/src/test/java/org/apache/rocketmq/store/ha/WaitNotifyObjectTest.java
##########
@@ -40,4 +40,57 @@ public void run() {
Assert.assertEquals(0, waitNotifyObject.waitingThreadTable.size());
}
+ @Test
+ public void allWaitForRunning() throws Exception {
+
+ final int threadNum = 5;
+ final long waitIntervalMs = 100L;
+ final CountDownLatch latch = new CountDownLatch(threadNum);
+ final WaitNotifyObject waitNotifyObject = new WaitNotifyObject() {
+ @Override
+ protected void onWaitEnd() {
+ latch.countDown();
+ }
+ };
+ long start = System.nanoTime();
+ for (int i = 0; i < threadNum; i++) {
+ Thread t = new Thread(new Runnable() {
+ @Override
+ public void run() {
+ waitNotifyObject.allWaitForRunning(waitIntervalMs);
+ }
+ });
+ t.start();
+ t.join();
+ }
+
+ latch.await();
+ long elapsed = (System.nanoTime() - start) / 1000000;
+ Assert.assertEquals(threadNum, waitNotifyObject.waitingThreadTable.size());
+ Assert.assertTrue(elapsed >= threadNum * waitIntervalMs);
+ }
+
+ @Test
+ public void wakeup() throws Exception {
+ final long waitIntervalMs = 3000L;
+ final long sleepMs = 500L;
+ final WaitNotifyObject waitNotifyObject = new WaitNotifyObject();
+ long start = System.nanoTime();
+ Thread t = new Thread(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ Thread.currentThread().sleep(sleepMs);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ waitNotifyObject.wakeup();
+ }
+ });
+ t.start();
+ waitNotifyObject.waitForRunning(waitIntervalMs);
+ long elapsed = (System.nanoTime() - start) / 1000000;
+ Assert.assertTrue(elapsed >= sleepMs && elapsed < waitIntervalMs);
+ }
+
Review comment:
This unit test has nothing to do with this pr, you can mention a pr again
--
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] vongosling commented on a change in pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
vongosling commented on a change in pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#discussion_r840168107
##########
File path: tools/src/test/java/org/apache/rocketmq/tools/command/broker/GetMasterSlaveDiffCommandTest.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.broker;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Options;
+import org.apache.commons.cli.PosixParser;
+import org.apache.rocketmq.client.exception.MQBrokerException;
+import org.apache.rocketmq.client.exception.MQClientException;
+import org.apache.rocketmq.common.protocol.body.ClusterInfo;
+import org.apache.rocketmq.common.protocol.route.BrokerData;
+import org.apache.rocketmq.remoting.exception.RemotingException;
+import org.apache.rocketmq.srvutil.ServerUtil;
+import org.apache.rocketmq.tools.admin.DefaultMQAdminExt;
+import org.apache.rocketmq.tools.command.SubCommandException;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.internal.util.collections.Sets;
+
+import java.lang.reflect.Field;
+import java.util.HashMap;
+import java.util.Set;
+
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
+
+public class GetMasterSlaveDiffCommandTest {
+
+ private GetMasterSlaveDiffCommand cmd;
+
+ private static DefaultMQAdminExt defaultMQAdminExt;
+
+ private static final String CLUSTER_NAME = "DefaultCluster";
+
+ private static final String BROKER_NAME = "broker-a";
+
+ @Before
+ public void before() throws NoSuchFieldException, IllegalAccessException, RemotingException, MQBrokerException,
+ InterruptedException, MQClientException {
+
+ ClusterInfo clusterInfo = new ClusterInfo();
+ HashMap<String, Set<String>> clusterMap = new HashMap<>(4);
+ clusterMap.put(CLUSTER_NAME, Sets.newSet(BROKER_NAME));
+ clusterInfo.setClusterAddrTable(clusterMap);
+
+ HashMap<Long, String> brokerMap = new HashMap<>(4);
+ brokerMap.put(0L, "127.0.0.1:10911");
+ BrokerData brokerData = new BrokerData(CLUSTER_NAME, BROKER_NAME, brokerMap);
+ HashMap<String, BrokerData> brokerAddrTable = new HashMap<>();
+ brokerAddrTable.put(BROKER_NAME, brokerData);
+ clusterInfo.setBrokerAddrTable(brokerAddrTable);
+
+ defaultMQAdminExt = new DefaultMQAdminExt();
+ defaultMQAdminExt = spy(defaultMQAdminExt);
+
+ cmd = new GetMasterSlaveDiffCommand();
+ Field field = cmd.getClass().getDeclaredField("defaultMQAdminExt");
+ field.setAccessible(true);
+ field.set(cmd, defaultMQAdminExt);
+
+ doReturn(clusterInfo).when(defaultMQAdminExt).examineBrokerClusterInfo();
+ doReturn(1024L).when(defaultMQAdminExt).getMasterSlaveDiff(anyString());
+ }
+
+ @After
+ public void after() {}
+
+ @Test
+ public void testExecute() throws SubCommandException, RemotingException, MQBrokerException, InterruptedException, MQClientException {
+ Options options = ServerUtil.buildCommandlineOptions(new Options());
+ String[] subargs = new String[] {"-b " + BROKER_NAME , "-c " + CLUSTER_NAME};
+ final CommandLine commandLine =
+ ServerUtil.parseCmdLine("mqadmin " + cmd.commandName(), subargs, cmd.buildCommandlineOptions(options), new PosixParser());
+ cmd.execute(commandLine, options, null);
Review comment:
Any assert for cmd execute?
--
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] XiaoyiPeng commented on a change in pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
XiaoyiPeng commented on a change in pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#discussion_r811748089
##########
File path: tools/src/main/java/org/apache/rocketmq/tools/command/broker/GetMasterSlaveDiffCommand.java
##########
@@ -0,0 +1,151 @@
+/*
+ * 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.broker;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.rocketmq.common.MixAll;
+import org.apache.rocketmq.common.protocol.body.ClusterInfo;
+import org.apache.rocketmq.common.protocol.route.BrokerData;
+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;
+
+import java.util.HashMap;
+import java.util.Set;
+
+public class GetMasterSlaveDiffCommand implements SubCommand {
+ @Override
+ public String commandName() {
+ return "getMasterSlaveDiff";
+ }
+
+ @Override
+ public String commandDesc() {
+ return "Get number of bytes that slave falls behind master.";
+ }
+
+ @Override
+ public Options buildCommandlineOptions(final Options options) {
+ Option opt = new Option("b", "brokerName", true, "Broker Name");
+ opt.setRequired(false);
+ options.addOption(opt);
+
+ opt = new Option("c", "clusterName", true, "Cluster Name");
+ opt.setRequired(false);
+ options.addOption(opt);
+
+ return options;
+ }
+
+ @Override
+ public void execute(final CommandLine commandLine, final Options options,
+ final RPCHook rpcHook) throws SubCommandException {
+ DefaultMQAdminExt defaultMQAdminExt = new DefaultMQAdminExt(rpcHook);
+ defaultMQAdminExt.setInstanceName(Long.toString(System.currentTimeMillis()));
+ try {
+ if (commandLine.hasOption('b')) {
+ String brokerAddr = commandLine.getOptionValue('b').trim();
+ defaultMQAdminExt.start();
+ printDiffForBroker(defaultMQAdminExt, brokerAddr);
+ } else if (commandLine.hasOption('c')) {
+ String clusterName = commandLine.getOptionValue('c').trim();
+ defaultMQAdminExt.start();
+ printDiffForCluster(defaultMQAdminExt, clusterName);
+ }
+ } catch (Exception e) {
+ throw new SubCommandException(this.getClass().getSimpleName() + " command failed", e);
+ } finally {
+ defaultMQAdminExt.shutdown();
+ }
+ }
+
+ private void printDiffForCluster(final DefaultMQAdminExt defaultMQAdminExt, String clusterName) throws SubCommandException {
+ ClusterInfo clusterInfo = null;
+ try {
+ clusterInfo = defaultMQAdminExt.examineBrokerClusterInfo();
+ } catch (Exception e) {
+ throw new SubCommandException(this.getClass().getSimpleName() + " command failed", e);
+ }
+
+ HashMap<String, Set<String>> clusterMap = clusterInfo.getClusterAddrTable();
+ Set<String> brokerNameSet = clusterMap.get(clusterName);
+ if (brokerNameSet == null || brokerNameSet.isEmpty()) {
+ System.out.printf("Can not find brokers for the cluster named %s!%n", clusterName);
+ return;
+ } else {
+ System.out.printf("%-24s %-24s %-24s %-14s%n",
+ "#Cluster Name",
+ "#Broker Name",
+ "#Broker Address(Master)",
+ "#Diff(Bytes)"
+ );
+ for (String brokerName : brokerNameSet) {
+ HashMap<String/* brokerName */, BrokerData> brokerAddrTable = clusterInfo.getBrokerAddrTable();
+ BrokerData brokerData;
+ if ((brokerData = brokerAddrTable.get(brokerName)) == null) {
+ continue;
+ } else {
+ String masterBrokerAddr = brokerData.getBrokerAddrs().get(MixAll.MASTER_ID);
+ if (StringUtils.isBlank(masterBrokerAddr))
+ continue;
+ try {
Review comment:
Thanks for reminding me. I've already optimized 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] panzhi33 commented on a change in pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
panzhi33 commented on a change in pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#discussion_r812599861
##########
File path: client/src/test/java/org/apache/rocketmq/client/impl/MQClientAPIImplTest.java
##########
@@ -476,4 +477,21 @@ public Object answer(InvocationOnMock invocationOnMock) throws Throwable {
int topicCnt = mqClientAPI.addWritePermOfBroker("127.0.0.1", "default-broker", 1000);
assertThat(topicCnt).isEqualTo(7);
}
+
+ @Test
+ public void testGetMasterSlaveDiff() throws Exception {
+ final long diff = 123L;
+ doAnswer(new Answer() {
+ @Override
+ public Object answer(InvocationOnMock invocationOnMock) throws Throwable {
+ final RemotingCommand response = RemotingCommand.createResponseCommand(RequestCode.GET_MASTER_SLAVE_DIFF, null);
+ response.setBody(String.valueOf(diff).getBytes(MixAll.DEFAULT_CHARSET));
+ response.setCode(ResponseCode.SUCCESS);
+ return response;
+ }
+ }).when(remotingClient).invokeSync(anyString(), any(RemotingCommand.class), anyLong());
+
+ long ret = mqClientAPI.getMasterSlaveDiff("127.0.0.1:10911", 1000);
+ assertThat(ret).isEqualTo(diff);
+ }
}
Review comment:
This unit test has nothing to do with this pr, you can mention a pr again
--
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] XiaoyiPeng edited a comment on pull request #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
XiaoyiPeng edited a comment on pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#issuecomment-1048467898
Hi @panzhi33 , @ni-ze , @Git-Yang ,
Could you help me to review this PR at your leisure? Thank you.
The related ISSUE #3681 is tagged with 4.9.3 milestone
--
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 #3746: [ISSUE #3681]Add the command to check master-slave synchronization status.
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3746:
URL: https://github.com/apache/rocketmq/pull/3746#issuecomment-1009854969
[](https://coveralls.io/builds/45525735)
Coverage decreased (-0.01%) to 53.086% when pulling **446c8c1e37d3c0b052186764d300cee1c38d16b9 on XiaoyiPeng:issue#3681** into **acac49f148b5c1070471d72a10b881cc15c58d6b 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