[GitHub] incubator-rocketmq pull request #157: [Rocketmq-277] fix get localhost throw...
Github user lizhanhui commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/157#discussion_r135698929
--- Diff: common/src/main/java/org/apache/rocketmq/common/MixAll.java ---
@@ -407,12 +408,50 @@ private static String localhost() {
InetAddress addr = InetAddress.getLocalHost();
return addr.getHostAddress();
} catch (Throwable e) {
+try {
+String candidatesHost = getLocalhostByNetworkInterface();
+if (candidatesHost != null)
+return candidatesHost;
+
+} catch (Exception ignored) {
+}
+
throw new RuntimeException("InetAddress
java.net.InetAddress.getLocalHost() throws UnknownHostException"
+ FAQUrl.suggestTodo(FAQUrl.UNKNOWN_HOST_EXCEPTION),
e);
}
}
+private static String getLocalhostByNetworkInterface() throws
SocketException {
--- End diff --
RemotingUtil has already had a similar method, aka, getLocalAddress, is
there any rational reason to create a duplicated one?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #157: [Rocketmq-277] fix get localhost throw java.n...
Github user vongosling commented on the issue: https://github.com/apache/incubator-rocketmq/pull/157 Thank you everyone. I will make some polish and merge this PR :-) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #160: [ROCKETMQ-284] ExpressionMessageFilter will p...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/160 [](https://coveralls.io/builds/13028196) Coverage increased (+0.02%) to 38.67% when pulling **0c9495f2458259bc419872a727403317de18bfb2 on vsair:ROCKETMQ-284** into **4c5e58b46acaf4541fbd741b97593618f027be2d on apache:develop**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #159: [ROCKETMQ-279] commit log data and con...
Github user fuyou001 commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/159#discussion_r135692614
--- Diff: store/src/main/java/org/apache/rocketmq/store/ConsumeQueue.java
---
@@ -84,6 +84,59 @@ public boolean load() {
return result;
}
+public void checkCommitLogAndConsumeQueueConsistent() {
+
+String queueDir = this.storePath
++ File.separator + topic
++ File.separator + queueId + File.separator;
+
+long maxOffsetInQueue = getMaxOffsetInQueue();
--- End diff --
refactor
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #157: [Rocketmq-277] fix get localhost throw...
Github user dongeforever commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/157#discussion_r135681743
--- Diff: common/src/test/java/org/apache/rocketmq/common/MixAllTest.java
---
@@ -93,4 +90,12 @@ public void testString2File() throws IOException {
MixAll.string2File("MixAll_testString2File", fileName);
assertThat(MixAll.file2String(fileName)).isEqualTo("MixAll_testString2File");
}
+
+@Test
+public void test_getLocalhostByNetworkInterface() throws Exception {
+Method method =
MixAll.class.getDeclaredMethod("getLocalhostByNetworkInterface");
+method.setAccessible(true);
+Object invoke = method.invoke(null);
+assertThat(invoke).isNotNull();
+}
--- End diff --
It may be better to avoid reflection.
Consider making this method be public or package-accessible
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #159: [ROCKETMQ-279] commit log data and con...
Github user zhouxinyu commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/159#discussion_r135688453
--- Diff: store/src/main/java/org/apache/rocketmq/store/ConsumeQueue.java
---
@@ -84,6 +84,59 @@ public boolean load() {
return result;
}
+public void checkCommitLogAndConsumeQueueConsistent() {
+
+String queueDir = this.storePath
++ File.separator + topic
++ File.separator + queueId + File.separator;
+
+long maxOffsetInQueue = getMaxOffsetInQueue();
--- End diff --
I would like to use `long lastRecordOffset = getMaxOffsetInQueue();`
instead, to avoid using too many `+1 or -1` in below code block. Actually,
`getMaxOffsetInQueue()` returns the next write position, not the position of
last consume queue record.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #158: [Rocketmq-281] add check policy for preventin...
Github user lindzh commented on the issue: https://github.com/apache/incubator-rocketmq/pull/158 After merge develop branch,all checks will pass --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #158: [Rocketmq-281] add check policy for pr...
Github user lindzh commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/158#discussion_r135681629
--- Diff:
broker/src/main/java/org/apache/rocketmq/broker/BrokerStartup.java ---
@@ -257,4 +268,28 @@ public static Options buildCommandlineOptions(final
Options options) {
return options;
}
+
+private static void checkMQAlreadyStart(String storePathRootDir)
throws IOException {
+File file = new
File(StorePathConfigHelper.getLockFile(storePathRootDir));
+MappedFile.ensureDirOK(file.getParent());
+
+final RandomAccessFile randomAccessFile = new
RandomAccessFile(file, "rw");
+FileLock lock = randomAccessFile.getChannel().tryLock(0, 1, false);
+if (lock == null || lock.isShared() || !lock.isValid()) {
+throw new RuntimeException("Lock failed,MQ already started");
+}
+
+
randomAccessFile.getChannel().write(ByteBuffer.wrap("lock".getBytes()));
+randomAccessFile.getChannel().force(true);
+
+Runtime.getRuntime().addShutdownHook(new Thread() {
--- End diff --
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #157: [Rocketmq-277] fix get localhost throw...
Github user zhouxinyu commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/157#discussion_r135678680
--- Diff: common/src/main/java/org/apache/rocketmq/common/MixAll.java ---
@@ -407,12 +408,50 @@ private static String localhost() {
InetAddress addr = InetAddress.getLocalHost();
return addr.getHostAddress();
} catch (Throwable e) {
+try {
+String candidatesHost = getLocalhostByNetworkInterface();
+if (candidatesHost != null)
+return candidatesHost;
+
+} catch (Exception ignored) {
+}
+
throw new RuntimeException("InetAddress
java.net.InetAddress.getLocalHost() throws UnknownHostException"
+ FAQUrl.suggestTodo(FAQUrl.UNKNOWN_HOST_EXCEPTION),
e);
}
}
+private static String getLocalhostByNetworkInterface() throws
SocketException {
+List candidatesHost = new ArrayList();
+Enumeration enumeration =
NetworkInterface.getNetworkInterfaces();
+
+while (enumeration.hasMoreElements()) {
+NetworkInterface networkInterface = enumeration.nextElement();
+if ("docker0".equals(networkInterface.getName()) ||
!networkInterface.isUp()) {
--- End diff --
Do we have a test in docker container for this method ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #158: [Rocketmq-281] add check policy for pr...
Github user zhouxinyu commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/158#discussion_r135678070
--- Diff:
broker/src/main/java/org/apache/rocketmq/broker/BrokerStartup.java ---
@@ -257,4 +268,28 @@ public static Options buildCommandlineOptions(final
Options options) {
return options;
}
+
+private static void checkMQAlreadyStart(String storePathRootDir)
throws IOException {
+File file = new
File(StorePathConfigHelper.getLockFile(storePathRootDir));
+MappedFile.ensureDirOK(file.getParent());
+
+final RandomAccessFile randomAccessFile = new
RandomAccessFile(file, "rw");
+FileLock lock = randomAccessFile.getChannel().tryLock(0, 1, false);
+if (lock == null || lock.isShared() || !lock.isValid()) {
+throw new RuntimeException("Lock failed,MQ already started");
+}
+
+
randomAccessFile.getChannel().write(ByteBuffer.wrap("lock".getBytes()));
+randomAccessFile.getChannel().force(true);
+
+Runtime.getRuntime().addShutdownHook(new Thread() {
--- End diff --
Also, release the file lock in `shutdown` of `MessageStore` is recommended.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #152: [ROCKETMQ-278]add clusterlist cmd by specifie...
Github user vsair commented on the issue: https://github.com/apache/incubator-rocketmq/pull/152 nice! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #152: [ROCKETMQ-278]add clusterlist cmd by specifie...
Github user coveralls commented on the issue: https://github.com/apache/incubator-rocketmq/pull/152 [](https://coveralls.io/builds/13014639) Coverage increased (+0.03%) to 38.68% when pulling **a674878c2125e305b6b3da293833b6583341cb1a on lindzh:add_clusterlist_cluster** into **4c5e58b46acaf4541fbd741b97593618f027be2d on apache:develop**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq pull request #145: [ROCKETMQ-264]Fix unit test cost too l...
Github user lindzh closed the pull request at: https://github.com/apache/incubator-rocketmq/pull/145 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] incubator-rocketmq issue #145: [ROCKETMQ-264]Fix unit test cost too long and...
Github user vongosling commented on the issue: https://github.com/apache/incubator-rocketmq/pull/145 @lindzh Thanks, I made some modifications and merged this PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
