[GitHub] incubator-rocketmq pull request #150: [ROCKETMQ-273] return an expression wh...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/150#discussion_r139873296
  
--- Diff: 
store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java ---
@@ -1094,34 +1090,15 @@ private boolean checkInDiskByCommitOffset(long 
offsetPy, long maxOffsetPy) {
 }
 
 private boolean isTheBatchFull(int sizePy, int maxMsgNums, int 
bufferTotal, int messageTotal, boolean isInDisk) {
-
-if (0 == bufferTotal || 0 == messageTotal) {
-return false;
-}
-
-if (maxMsgNums <= messageTotal) {
-return true;
-}
-
-if (isInDisk) {
-if ((bufferTotal + sizePy) > 
this.messageStoreConfig.getMaxTransferBytesOnMessageInDisk()) {
-return true;
-}
-
-if (messageTotal > 
this.messageStoreConfig.getMaxTransferCountOnMessageInDisk() - 1) {
-return true;
-}
-} else {
-if ((bufferTotal + sizePy) > 
this.messageStoreConfig.getMaxTransferBytesOnMessageInMemory()) {
-return true;
-}
-
-if (messageTotal > 
this.messageStoreConfig.getMaxTransferCountOnMessageInMemory() - 1) {
-return true;
-}
-}
-
-return false;
+return   (0 != bufferTotal && 0 != messageTotal)
+&&(
--- End diff --

Human readable problem


---

[GitHub] incubator-rocketmq pull request #152: [ROCKETMQ-278] Add clusterlist cmd by ...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/152#discussion_r139871853
  
--- Diff: 
namesrv/src/main/java/org/apache/rocketmq/namesrv/routeinfo/RouteInfoManager.java
 ---
@@ -63,11 +65,36 @@ public RouteInfoManager() {
 this.filterServerTable = new HashMap<String, List>(256);
 }
 
-public byte[] getAllClusterInfo() {
-ClusterInfo clusterInfoSerializeWrapper = new ClusterInfo();
-
clusterInfoSerializeWrapper.setBrokerAddrTable(this.brokerAddrTable);
-
clusterInfoSerializeWrapper.setClusterAddrTable(this.clusterAddrTable);
-return clusterInfoSerializeWrapper.encode();
+public byte[] getAllClusterInfo(String cluster) {
--- End diff --

I agree @shroman here


---

[GitHub] incubator-rocketmq-externals issue #29: [ROCKETMQ-193] Develop rocketmq-redi...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq-externals/pull/29
  
 Write necessary unit-test 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.


Do you have test module in this replicator ?


---

[GitHub] incubator-rocketmq-externals issue #23: [ROCKETMQ-193] init subproject: rock...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq-externals/pull/23
  
@Zhang-Ke Thanks, It is really good practice when discarded deserted pr.


---

[GitHub] incubator-rocketmq-externals issue #29: update redis-replicator to 2.3.3 , f...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq-externals/pull/29
  
Follow this checklist to help us incorporate your contribution quickly and 
easily:

- [ ]  Make sure there is a [JIRA 
issue](https://issues.apache.org/jira/projects/ROCKETMQ/issues/) filed for the 
change (usually before you start working on it). Trivial changes like typos do 
not require a JIRA issue. Your pull request should address just this issue, 
without pulling in other changes - one PR resolves one issue. 
- [ ] Format the pull request title like `[ROCKETMQ-XXX] Fix 
UnknownException when host config not exist`. Each commit in the pull request 
should have a meaningful subject line and body.
- [ ] Write a pull request description that is detailed enough to 
understand what the pull request does, how, and why.
- [ ] Write necessary unit-test 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/incubator-rocketmq/tree/master/test).
- [ ]  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).



---

[GitHub] incubator-rocketmq-externals issue #29: update redis-replicator to 2.3.3 , f...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq-externals/pull/29
  
@leonchen83 Thanks for your contribution. BTW, have you test it in your 
local environment ?


---

[GitHub] incubator-rocketmq issue #161: 注释

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/161
  
@liucyu If no update for this PR, we will close it. 


---
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 #160: [ROCKETMQ-284] ExpressionMessageFilter...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/160#discussion_r135768140
  
--- Diff: 
broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java
 ---
@@ -79,24 +79,17 @@
 try {
 StoreHost = new InetSocketAddress(InetAddress.getLocalHost(), 
8123);
 } catch (UnknownHostException e) {
-e.printStackTrace();
 }
 try {
 BornHost = new 
InetSocketAddress(InetAddress.getByName("127.0.0.1"), 0);
 } catch (UnknownHostException e) {
-e.printStackTrace();
 }
 }
 
 @Before
-public void init() {
+public void init() throws Exception {
 filterManager = ConsumerFilterManagerTest.gen(topicCount, 
msgPerTopic);
-try {
-master = gen(filterManager);
-} catch (Exception e) {
-e.printStackTrace();
-assertThat(true).isFalse();
-}
--- End diff --

Excellent !


---
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 #160: [ROCKETMQ-284] ExpressionMessageFilter...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/160#discussion_r135722707
  
--- Diff: 
broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java
 ---
@@ -369,4 +367,36 @@ public void testGetMessage_withFilterBitMap() {
 }
 }
 }
+
+@Test
+public void testGetMessage_withFilter_checkTagsCode() {
+try {
+putMsg(master, topicCount, msgPerTopic);
+// sleep to wait for consume queue has been constructed.
+Thread.sleep(200);
+} catch (Exception e) {
--- End diff --

Could we using expected  for exception test ?


---
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 #160: [ROCKETMQ-284] ExpressionMessageFilter...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/160#discussion_r135724026
  
--- Diff: 
broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java
 ---
@@ -369,4 +367,36 @@ public void testGetMessage_withFilterBitMap() {
 }
 }
 }
+
+@Test
+public void testGetMessage_withFilter_checkTagsCode() {
+try {
+putMsg(master, topicCount, msgPerTopic);
+// sleep to wait for consume queue has been constructed.
--- End diff --

Must we use sleep to replace sleep ? Now ,we have polished our unit test 
within 3 minutes, thoughts @shroman 


---
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...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/157#discussion_r135705543
  
--- 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 --

I have marked it using FIXME label. reverse logic comparing to RemotingUtil 
method, we could consider to refactor it in RocketMQ 5.0.



---
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...

[vongosling]

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 #145: [ROCKETMQ-264]Fix unit test cost too long and...

[vongosling]

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.
---

[GitHub] incubator-rocketmq issue #145: [ROCKETMQ-264]Fix unit test cost too long and...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/145
  
@Jaskey @vsair Looking forward to your opinion about 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 #146: [ROCKETMQ-265] fix consume queue’s data may...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/146
  
LGTM, now~


---
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...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/145#discussion_r134941453
  
--- Diff: 
client/src/test/java/org/apache/rocketmq/client/consumer/rebalance/AllocateMessageQueueConsitentHashTest.java
 ---
@@ -92,9 +92,9 @@ public void testAllocate2() {
 
 @Test
 public void testRun100RandomCase() {
-for (int i = 0; i < 100; i++) {
-int consumerSize = new Random().nextInt(200) + 1;//1-200
-int queueSize = new Random().nextInt(100) + 1;//1-100
+for (int i = 0; i < 10; i++) {
+int consumerSize = new Random().nextInt(20) + 1;//1-20
--- End diff --

@shroman I would like to verify the program correction though the specific 
trick not iteration count, especially in unit-test, thoughts ?


---
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 #146: [ROCKETMQ-265] fix consume queue’s d...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/146#discussion_r134940285
  
--- Diff: 
broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java
 ---
@@ -238,7 +238,7 @@ public void 
testGetMessage_withFilterBitMapAndConsumerChanged() {
 
 // sleep to wait for consume queue has been constructed.
 try {
-Thread.sleep(1000);
+Thread.sleep(2001);
 } catch (InterruptedException e) {
 e.printStackTrace();
--- End diff --

I would like to recommend to remove it in test module


---
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 #146: [ROCKETMQ-265] fix consume queue’s d...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/146#discussion_r134940242
  
--- Diff: store/src/main/java/org/apache/rocketmq/store/ConsumeQueue.java 
---
@@ -446,6 +446,13 @@ private boolean putMessagePositionInfo(final long 
offset, final int size, final
 
 if (cqOffset != 0) {
 long currentLogicOffset = mappedFile.getWrotePosition() + 
mappedFile.getFileFromOffset();
+
+if (expectLogicOffset < currentLogicOffset) {
+log.warn("build consume queue idempotent, 
expectLogicOffset: {} currentLogicOffset: {} Topic: {} QID: {} Diff: {}",
--- End diff --

How to understand the meaning of building consume queue idempotent？


---
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...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/145#discussion_r133153891
  
--- Diff: 
broker/src/test/java/org/apache/rocketmq/broker/filter/MessageStoreWithFilterTest.java
 ---
@@ -76,6 +86,24 @@
 }
 }
 
+@Before
+public void init() {
+filterManager = ConsumerFilterManagerTest.gen(topicCount, 
msgPerTopic);
+try {
+master = gen(filterManager);
+} catch (Exception e) {
+e.printStackTrace();
--- End diff --

redundant code, when you use assert in your exception scenario, right ?


---
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-externals issue #23: [ROCKETMQ-193] init subproject: rock...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq-externals/pull/23
  
@Zhang-Ke Could you remove author info and follow our codestyle, such as 
add license. if have any question, please let me know :-)


---
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 #134: [ROCKETMQ-44] Duplicated Codes in DefaultMess...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/134
  
@Ritabrata-TW Thanks for your attention for rocketmq community, whats' your 
scenario when using apache rocketmq in your company :-)


---
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-externals issue #19: [DISCUSS]Go-Client:Detail Design

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq-externals/pull/19
  
 How are things going, now :-)


---
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-externals issue #26: MySQL:To prepare release mysql repli...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq-externals/pull/26
  
LGTM


---
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 #146: [ROCKETMQ-265] fix consume queue’s data may...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/146
  
@fuyou001 Could you verify your polish using unit-test ?


---
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 #134: [ROCKETMQ-44] Duplicated Codes in DefaultMess...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/134
  
@Ritabrata-TW I found so many code format in your PR, could you import our 
code style file as instruction, 
http://rocketmq.incubator.apache.org/docs/code-guidelines/


---
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...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/145
  
How long it would take in your computer ? @Jaskey @vsair, please double 
check this optimized ut.


---
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 #126: [ROCKETMQ-231] fix pull consumer pull result ...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/126
  
Thanks @lindzh, i have merged it. Please close 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 #132: [ROCKETMQ-248] make ConsumeFromWhere work rig...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/132
  
@evthoriz we usually need some measure to verify our correction, no matter 
what you do. IMO, unit test is a effective practice when we coding. Looking 
forward to hear from you again :-)


---
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 #142: [ROCKETMQ-255] Fix offsetStore being null aft...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/142
  
I created this thread just to address some unit test naming problem. Do we 
need normalize here ? naming are various nowadays, IMO, methodTest_condition or 
testMethond_condition may be a good practice, thoughts?


---
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 #85: [ROCKETMQ-158]Remove logback dependency for ro...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/85
  
+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 issue #141: [ROCKETMQ-254] fix logger appender unit test ...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/141
  
LGTM


---
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 #134: [ROCKETMQ-44] Duplicated Codes in DefaultMess...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/134
  
@Ritabrata-TW thanks, I will take a close look it later :-)


---
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 #126: [ROCKETMQ-231] fix pull consumer pull result ...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/126
  
Now, LGTM


---
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 #120: [ROCKETMQ-224] add rocketmq client log4j2 log...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/120
  
Now, LGTM. this pr also fix the rocketmq's logger appender bug when using 
any concrete implementation, no matter log4j, log4j2 or logback. 


---
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 #120: [ROCKETMQ-224] add rocketmq client log...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r130550009
  
--- Diff: 
client/src/test/java/org/apache/rocketmq/client/log/ClientLogTest.java ---
@@ -32,15 +33,19 @@
 LOG_DIR = System.getProperty(CLIENT_LOG_ROOT, 
"${user.home}/logs/rocketmqlogs");
 }
 
+// FIXME: Workarond for concret implementation for slf4j, is there any 
better solution for all slf4j implementations in one class ? 2017/8/1
--- End diff --

we'd better remember this task and fix it later.


---
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 #134: Rito - #44 - Extracting out duplicated...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/134#discussion_r130526510
  
--- Diff: 
store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java ---
@@ -745,17 +751,7 @@ public long getMessageStoreTimeStamp(String topic, int 
queueId, long consumeQueu
 ConsumeQueue logicQueue = this.findConsumeQueue(topic, queueId);
 if (logicQueue != null) {
 SelectMappedBufferResult result = 
logicQueue.getIndexBuffer(consumeQueueOffset);
-if (result != null) {
-try {
-final long phyOffset = 
result.getByteBuffer().getLong();
-final int size = result.getByteBuffer().getInt();
-long storeTime = 
this.getCommitLog().pickupStoreTimestamp(phyOffset, size);
-return storeTime;
-} catch (Exception ignored) {
-} finally {
-result.release();
-}
-}
+long storeTime = getStoreTime(result);
--- End diff --

We appreciate your contribution for rocketmq commuity, could you use unit 
test to verify your refactoring


---
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 #134: Rito - #44 - Extracting out duplicated code f...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/134
  
@Ritabrata-TW I am not understand your refer "Story number 44 from the 
Kanban Board", could you detail it ?


---
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 #141: [ROCKETMQ-254] fix logger appender uni...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/141#discussion_r130525133
  
--- Diff: 
logappender/src/main/java/org/apache/rocketmq/logappender/common/ProducerInstance.java
 ---
@@ -61,7 +68,7 @@ public static MQProducer getInstance(String 
nameServerAddress, String group) thr
 defaultMQProducer.setNamesrvAddr(nameServerAddress);
 MQProducer beforeProducer = null;
 //cas put producer
--- End diff --

what's the purpose for this comment :-)


---
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 #58: Update runbroker.sh

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/58
  
if any update for this pr, we will close it~


---
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 #71: add telnet CMD to nameServer

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/71
  
if any update for this pr, we will close it~


---
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 #99: Correct comment information

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/99
  
if any update for this pr, we will close it~


---
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 #124: ROCKETMQ-213 :

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/124
  
if any update for this pr, we will close it~


---
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 #122: ROCKETMQ-214

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/122
  
if any update for this pr, we will close it~


---
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 #125: ROCKETMQ-205 :

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/125
  
@lqjack what's your opinion


---
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 #134: Rito - #44 - Extracting out duplicated code f...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/134
  
@Ritabrata-TW Could you modify PR subject conforming to our norm :-) 


---
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 #118: [ROCKETMQ-28] Encrypt transmission layer.

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/118
  
LGTM


---
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 #126: [ROCKETMQ-231] fix pull consumer pull ...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/126#discussion_r125432988
  
--- Diff: 
store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java ---
@@ -1110,7 +1110,7 @@ private boolean isTheBatchFull(int sizePy, int 
maxMsgNums, int bufferTotal, int
 return false;
 }
 
-if ((messageTotal + 1) >= maxMsgNums) {
--- End diff --

why change to -1 in your new change:-)


---
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 #3: [ROCKETMQ-6] Use logger for exceptions instead ...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/3
  
any update?  or close it :-)


---
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 #3: [ROCKETMQ-6] Use logger for exceptions i...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/3#discussion_r93905062
  
--- Diff: 
rocketmq-remoting/src/main/java/com/alibaba/rocketmq/remoting/common/ServiceThread.java
 ---
@@ -86,7 +86,7 @@ public void stop() {
 
 public void stop(final boolean interrupt) {
 this.stopped = true;
-STLOG.info("stop thread " + this.getServiceName() + " interrupt " 
+ interrupt);
+log.info("stop thread " + this.getServiceName() + " interrupt " + 
interrupt);
--- End diff --

the same 


---
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 #3: [ROCKETMQ-6] Use logger for exceptions i...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/3#discussion_r93905067
  
--- Diff: 
rocketmq-remoting/src/main/java/com/alibaba/rocketmq/remoting/common/ServiceThread.java
 ---
@@ -101,7 +101,7 @@ public void stop(final boolean interrupt) {
 
 public void makeStop() {
 this.stopped = true;
-STLOG.info("makestop thread " + this.getServiceName());
+log.info("makestop thread " + this.getServiceName());
--- End diff --

the same 


---
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 #49: [ROCKETMQ-72] DefaultMessageStore cannot be pr...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/49
  
What's going on 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 pull request #126: [ROCKETMQ-231] fix pull consumer pull ...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/126#discussion_r123730992
  
--- Diff: 
test/src/test/java/org/apache/rocketmq/test/client/consumer/pull/PullSizeTest.java
 ---
@@ -0,0 +1,135 @@
+/*
+ * 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.test.client.consumer.pull;
+
+import org.apache.log4j.Logger;
+import org.apache.rocketmq.client.consumer.DefaultMQPullConsumer;
+import org.apache.rocketmq.client.consumer.PullResult;
+import org.apache.rocketmq.client.exception.MQClientException;
+import org.apache.rocketmq.client.producer.DefaultMQProducer;
+import org.apache.rocketmq.client.producer.SendResult;
+import org.apache.rocketmq.client.producer.SendStatus;
+import org.apache.rocketmq.common.message.Message;
+import org.apache.rocketmq.common.message.MessageExt;
+import org.apache.rocketmq.common.message.MessageQueue;
+import org.apache.rocketmq.remoting.common.RemotingHelper;
+import org.apache.rocketmq.test.base.BaseConf;
+import org.apache.rocketmq.test.base.IntegrationTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.nio.channels.Pipe;
+import java.util.*;
+
+public class PullSizeTest extends BaseConf {
+
+private static Logger logger = Logger.getLogger(PullSizeTest.class);
+
+private static final Map<MessageQueue, Long> OFFSE_TABLE = new 
HashMap<MessageQueue, Long>();
+
+public static final String PULL_SIZE_TOPIC = "TopicPullTest";
+
+public static final String PULL_SIZE_GROUP = "pullSizeTest";
+
+public boolean send() throws MQClientException, InterruptedException {
+DefaultMQProducer producer = new 
DefaultMQProducer(PULL_SIZE_GROUP);
+producer.setNamesrvAddr(nsAddr);
+producer.start();
+int successCount = 0;
+for (int i = 0; i < 1000; i++) {
+try {
+Message msg = new Message(PULL_SIZE_TOPIC, "TagA", 
("RocketMQ pull size test index " + 
i).getBytes(RemotingHelper.DEFAULT_CHARSET));
+SendResult sendResult = producer.send(msg);
+if (sendResult.getSendStatus().equals(SendStatus.SEND_OK)) 
{
+successCount++;
+}
+} catch (Exception e) {
+e.printStackTrace();
+Thread.sleep(500);
+}
+}
+producer.shutdown();
+if (successCount > 800) {
--- End diff --

why use this magic number 800 in your if statement


---
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 #126: [ROCKETMQ-231] fix pull consumer pull ...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/126#discussion_r123730667
  
--- Diff: 
test/src/test/java/org/apache/rocketmq/test/client/consumer/pull/PullSizeTest.java
 ---
@@ -0,0 +1,135 @@
+/*
+ * 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.test.client.consumer.pull;
+
+import org.apache.log4j.Logger;
+import org.apache.rocketmq.client.consumer.DefaultMQPullConsumer;
+import org.apache.rocketmq.client.consumer.PullResult;
+import org.apache.rocketmq.client.exception.MQClientException;
+import org.apache.rocketmq.client.producer.DefaultMQProducer;
+import org.apache.rocketmq.client.producer.SendResult;
+import org.apache.rocketmq.client.producer.SendStatus;
+import org.apache.rocketmq.common.message.Message;
+import org.apache.rocketmq.common.message.MessageExt;
+import org.apache.rocketmq.common.message.MessageQueue;
+import org.apache.rocketmq.remoting.common.RemotingHelper;
+import org.apache.rocketmq.test.base.BaseConf;
+import org.apache.rocketmq.test.base.IntegrationTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.nio.channels.Pipe;
+import java.util.*;
+
+public class PullSizeTest extends BaseConf {
+
+private static Logger logger = Logger.getLogger(PullSizeTest.class);
--- End diff --

Could we remove logger in unit test ?


---
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 #126: [ROCKETMQ-231] fix pull consumer pull ...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/126#discussion_r123731300
  
--- Diff: 
test/src/test/java/org/apache/rocketmq/test/client/consumer/pull/PullSizeTest.java
 ---
@@ -0,0 +1,135 @@
+/*
+ * 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.test.client.consumer.pull;
+
+import org.apache.log4j.Logger;
+import org.apache.rocketmq.client.consumer.DefaultMQPullConsumer;
+import org.apache.rocketmq.client.consumer.PullResult;
+import org.apache.rocketmq.client.exception.MQClientException;
+import org.apache.rocketmq.client.producer.DefaultMQProducer;
+import org.apache.rocketmq.client.producer.SendResult;
+import org.apache.rocketmq.client.producer.SendStatus;
+import org.apache.rocketmq.common.message.Message;
+import org.apache.rocketmq.common.message.MessageExt;
+import org.apache.rocketmq.common.message.MessageQueue;
+import org.apache.rocketmq.remoting.common.RemotingHelper;
+import org.apache.rocketmq.test.base.BaseConf;
+import org.apache.rocketmq.test.base.IntegrationTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.nio.channels.Pipe;
+import java.util.*;
+
+public class PullSizeTest extends BaseConf {
+
+private static Logger logger = Logger.getLogger(PullSizeTest.class);
+
+private static final Map<MessageQueue, Long> OFFSE_TABLE = new 
HashMap<MessageQueue, Long>();
+
+public static final String PULL_SIZE_TOPIC = "TopicPullTest";
+
+public static final String PULL_SIZE_GROUP = "pullSizeTest";
+
+public boolean send() throws MQClientException, InterruptedException {
+DefaultMQProducer producer = new 
DefaultMQProducer(PULL_SIZE_GROUP);
+producer.setNamesrvAddr(nsAddr);
+producer.start();
+int successCount = 0;
+for (int i = 0; i < 1000; i++) {
+try {
+Message msg = new Message(PULL_SIZE_TOPIC, "TagA", 
("RocketMQ pull size test index " + 
i).getBytes(RemotingHelper.DEFAULT_CHARSET));
+SendResult sendResult = producer.send(msg);
+if (sendResult.getSendStatus().equals(SendStatus.SEND_OK)) 
{
+successCount++;
+}
+} catch (Exception e) {
+e.printStackTrace();
+Thread.sleep(500);
+}
+}
+producer.shutdown();
+if (successCount > 800) {
+return true;
+} else {
+return false;
+}
+}
+
+
+public void pullMsg() throws MQClientException {
+boolean result = false;
+DefaultMQPullConsumer consumer = new 
DefaultMQPullConsumer(PULL_SIZE_GROUP);
+consumer.setNamesrvAddr(nsAddr);
+consumer.start();
+Set mqs = 
consumer.fetchSubscribeMessageQueues(PULL_SIZE_TOPIC);
+for (MessageQueue mq : mqs) {
+if (result) {
+break;
+}
+try {
+PullResult pullResult = consumer.pull(mq, null, 
getMessageQueueOffset(mq), 32);
+putMessageQueueOffset(mq, pullResult.getNextBeginOffset());
+switch (pullResult.getPullStatus()) {
+case FOUND:
+List msgFoundList = 
pullResult.getMsgFoundList();
+if (msgFoundList != null) {
+logger.info("[RECV] received msg queue:" + 
mq.getBrokerName() + "-" + mq.getQueueId());
+result |= msgFoundList.size() >= 32;
+}
+break;
+case NO_MATCHED_MSG:
+break;
+

[GitHub] incubator-rocketmq pull request #120: [ROCKETMQ-224] add rocketmq client log...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r123713735
  
--- Diff: client/pom.xml ---
@@ -37,13 +37,31 @@
 ${project.groupId}
 rocketmq-common
 
+
 
 org.slf4j
 slf4j-api
 
+
 
 org.apache.commons
 commons-lang3
 
+
+
+org.apache.logging.log4j
+log4j-core
+test
+
+
+org.apache.logging.log4j
+log4j-api
+test
+
+
+org.apache.logging.log4j
+log4j-slf4j-impl
--- End diff --

Could we  put log4j2 test in rocketmq example module ?


---
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 #120: [ROCKETMQ-224] add rocketmq client log...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r123714561
  
--- Diff: pom.xml ---
@@ -638,6 +638,16 @@
 log4j-core
 2.7
 
+
+org.apache.logging.log4j
--- End diff --

Why could i depend log4j2 core directly in dependency management?


---
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-externals issue #24: RocketMQ-MySQL 1.0-snapshot

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq-externals/pull/24
  
I will merge this PR and polish it later


---
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-site pull request #18: [ROCKETMQ-194]Add logappender exam...

[vongosling]

Github user vongosling commented on a diff in the pull request:


https://github.com/apache/incubator-rocketmq-site/pull/18#discussion_r120898962
  
--- Diff: _docs/20-logappender-example.md ---
@@ -0,0 +1,80 @@
+---
+title: "Logappender Example"
+permalink: /docs/logappender-example/
+excerpt: "How to use logappender in RocketMQ."
+modified: 2017-06-08T21:01:43-04:00
+---
+
+{% include toc %}
+
+When we use rocketmq as a part of big data processing or other 
cases,bussiness also need to put log data to rocketmq.Rocketmq logappender 
provides log4j appender,logback appender and log4j2 appender for bussiness to 
use,below is some example.
+
+ log4j 
+
+When using log4j properties config file,config as below.
+
+```

+log4j.appender.mq=org.apache.rocketmq.logappender.log4j.RocketmqLog4jAppender
+log4j.appender.mq.Tag=yourTag
+log4j.appender.mq.Topic=yourLogTopic
+log4j.appender.mq.ProducerGroup=yourLogGroup
+log4j.appender.mq.NameServerAddress=yourRocketmqNameserverAddress
+log4j.appender.mq.layout=org.apache.log4j.PatternLayout
+log4j.appender.mq.layout.ConversionPattern=%d{-MM-dd HH:mm:ss} %-4r 
[%t] (%F:%L) %-5p - %m%n
+```
+
+When using log4j xml config file,config it as this and also add a async 
appender:
+
+```
+
+
--- End diff --

capitalize the first letter in name?


---
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-externals pull request #24: RocketMQ-MySQL 1.0-snapshot

[vongosling]

Github user vongosling commented on a diff in the pull request:


https://github.com/apache/incubator-rocketmq-externals/pull/24#discussion_r121606021
  
--- Diff: 
rocketmq-mysql/src/main/java/org/apache/rocketmq/mysql/Config.java ---
@@ -0,0 +1,132 @@
+/*
+ * 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.mysql;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.reflect.Method;
+import java.util.Properties;
+
+/**
+ *
--- End diff --

Remove javadoc


---
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-externals pull request #24: RocketMQ-MySQL 1.0-snapshot

[vongosling]

Github user vongosling commented on a diff in the pull request:


https://github.com/apache/incubator-rocketmq-externals/pull/24#discussion_r121605897
  
--- Diff: rocketmq-mysql/pom.xml ---
@@ -0,0 +1,163 @@
+
+http://maven.apache.org/POM/4.0.0;
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+4.0.0
+
+org.apache
+rocketmq-mysql
+1.0-SNAPSHOT
+
+
+UTF-8
+false
+true
+1.7
+1.7
+1.7.0
+4.0.0-incubating
+
+
+
+
+
+org.apache.rocketmq
+rocketmq-client
+${rocketmq.version}
+
+
+com.zendesk
+open-replicator
+1.6.0
+
+
+mysql
+mysql-connector-java
+5.1.39
+
+
+org.slf4j
+slf4j-api
+1.7.5
+
+
+ch.qos.logback
+logback-classic
+1.0.13
+
+
+ch.qos.logback
+logback-core
+1.0.13
+
+
+com.alibaba
--- End diff --

recommend to remove druid dependency


---
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-externals pull request #24: RocketMQ-MySQL 1.0-snapshot

[vongosling]

Github user vongosling commented on a diff in the pull request:


https://github.com/apache/incubator-rocketmq-externals/pull/24#discussion_r121605783
  
--- Diff: rocketmq-mysql/pom.xml ---
@@ -0,0 +1,163 @@
+
+http://maven.apache.org/POM/4.0.0;
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+4.0.0
+
+org.apache
+rocketmq-mysql
+1.0-SNAPSHOT
+
+
+UTF-8
+false
+true
+1.7
+1.7
+1.7.0
+4.0.0-incubating
+
+
+
+
+
+org.apache.rocketmq
+rocketmq-client
+${rocketmq.version}
+
+
+com.zendesk
+open-replicator
+1.6.0
+
+
+mysql
+mysql-connector-java
+5.1.39
+
+
+org.slf4j
--- End diff --

Many question about your log dependency


---
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-externals pull request #24: RocketMQ-MySQL 1.0-snapshot

[vongosling]

Github user vongosling commented on a diff in the pull request:


https://github.com/apache/incubator-rocketmq-externals/pull/24#discussion_r121604785
  
--- Diff: rocketmq-mysql/.gitignore ---
@@ -0,0 +1,17 @@
+*dependency-reduced-pom.xml
+.classpath
+.project
+.settings/
+target/
+devenv
+*.log*
+*.iml
+.idea/
+*.versionsBackup
+*bin
+!NOTICE-BIN
+!LICENSE-BIN
+.DS_Store
+将删除 .gitignore
--- End diff --

please remove it


---
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 #111: [ROCKETMQ-28] Add TLS support to provi...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/111#discussion_r120903001
  
--- Diff: 
remoting/src/main/java/org/apache/rocketmq/remoting/netty/SslHelper.java ---
@@ -0,0 +1,110 @@
+/*
+ * 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.remoting.netty;
+
+import io.netty.handler.ssl.ClientAuth;
+import io.netty.handler.ssl.SslContext;
+import io.netty.handler.ssl.SslContextBuilder;
+import io.netty.handler.ssl.SslProvider;
+import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
+import io.netty.handler.ssl.util.SelfSignedCertificate;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.InputStream;
+import java.security.cert.CertificateException;
+import java.util.Properties;
+import javax.net.ssl.SSLException;
+
+public class SslHelper {
+
+public static SslContext buildSslContext(boolean forClient) throws 
SSLException, CertificateException {
+
+File configFile = new File(NettySystemConfig.sslConfigFile);
+boolean testMode = !(configFile.exists() && configFile.isFile() && 
configFile.canRead());
+Properties properties = null;
+
+if (!testMode) {
+properties = new Properties();
+InputStream inputStream = null;
+try {
+inputStream = new FileInputStream(configFile);
--- End diff --

@lizhanhui 
@Jaskey  agreed, client must be compatible with JDK 1.6 considering 
RocketMQ's massive user:-)


---
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 #111: [ROCKETMQ-28] Add TLS support to provi...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/111#discussion_r120901312
  
--- Diff: 
remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingAbstract.java
 ---
@@ -88,6 +89,11 @@
 protected Pair<NettyRequestProcessor, ExecutorService> 
defaultRequestProcessor;
 
 /**
+ * SSL Context.
--- End diff --

No need to use the same text, we'd better to remove this javadoc if we 
could not further describe this class :-)


---
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 #112: [ROCKETMQ-219] Add batch example

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/112
  
+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 issue #113: [ROCKETMQ-218] README.md update

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/113
  
+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-site issue #16: [ROCKETMQ-221] Website beta version finis...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq-site/pull/16
  
+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-site issue #16: [ROCKETMQ-221] Website beta version finis...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq-site/pull/16
  
Could you resolve the conflict firstly


---
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-site issue #15: [ROCKETMQ-217] Polish scheduled message e...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq-site/pull/15
  
LGTM, +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-site issue #13: Polish motivation

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq-site/pull/13
  
Very pleased to see your contribution.But could you please create an issue 
at 
https://issues.apache.org/jira/browse/ROCKETMQ/?selectedTab=com.atlassian.jira.jira-projects-plugin:issues-panel?
 And also rename this pull request title, like, [ROCKETMQ-XXX] as the issue id?


---
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-site issue #14: Website polish

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq-site/pull/14
  
Very pleased to see your contribution.But could you please create an issue 
at 
https://issues.apache.org/jira/browse/ROCKETMQ/?selectedTab=com.atlassian.jira.jira-projects-plugin:issues-panel?
 And also rename this pull request title, like, [ROCKETMQ-XXX] as the issue id?


---
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 #105: [ROCKETMQ-200]-Cluster name is always ...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/105#discussion_r118622000
  
--- Diff: 
namesrv/src/main/java/org/apache/rocketmq/namesrv/routeinfo/RouteInfoManager.java
 ---
@@ -125,11 +125,7 @@ public RegisterBrokerResult registerBroker(
 BrokerData brokerData = 
this.brokerAddrTable.get(brokerName);
 if (null == brokerData) {
 registerFirst = true;
-brokerData = new BrokerData();
-brokerData.setBrokerName(brokerName);
-HashMap<Long, String> brokerAddrs = new HashMap<Long, 
String>();
-brokerData.setBrokerAddrs(brokerAddrs);
-
+brokerData = new BrokerData(clusterName, brokerName, 
new HashMap<Long, String>());
--- End diff --

clusterName can be null here ?


---
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 #105: [ROCKETMQ-200]-Cluster name is always ...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/105#discussion_r118621860
  
--- Diff: 
common/src/main/java/org/apache/rocketmq/common/protocol/route/BrokerData.java 
---
@@ -15,9 +15,7 @@
  * limitations under the License.
  */
 
-/**
- * $Id: BrokerData.java 1835 2013-05-16 02:00:50Z vintagew...@apache.org $
- */
+
--- End diff --

excellent job


---
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 #101: [ROCKETMQ-194] log appender support

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/101#discussion_r115887404
  
--- Diff: 
logappender/src/main/java/org/apache/rocketmq/logappender/log4j2/RocketmqLog4j2Appender.java
 ---
@@ -0,0 +1,232 @@
+/*
+ * 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.logappender.log4j2;
+
+import org.apache.logging.log4j.core.config.plugins.Plugin;
+import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute;
+import org.apache.logging.log4j.core.config.plugins.PluginBuilderFactory;
+import org.apache.logging.log4j.core.config.plugins.PluginElement;
+import org.apache.rocketmq.common.message.Message;
+import org.apache.rocketmq.logappender.common.ProducerInstance;
+import org.apache.logging.log4j.core.ErrorHandler;
+import org.apache.logging.log4j.core.Filter;
+import org.apache.logging.log4j.core.Layout;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+import 
org.apache.logging.log4j.core.config.plugins.validation.constraints.Required;
+import org.apache.logging.log4j.core.layout.SerializedLayout;
+import org.apache.rocketmq.client.producer.MQProducer;
+
+import java.io.Serializable;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Log4j2 Appender Component
+ */
+@Plugin(
+name = "Rocketmq",
+category = "Core",
--- End diff --

Core or core ?


---
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 #97: [ROCKETMQ-189] Misleading tip on consumeTimest...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/97
  
IMO,if we could polish here. we 'd better tell what's wrong with your input 
and what's a valid input ,right ?


---
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 #80: [ROCKETMQ-114] Add javadoc to codebase

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/80
  
What's wrong with this PR. if no other polish, suggest close it:-)


---
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 #93: [ROCKETMQ-178] Fix -p -m options

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/93
  
Any trouble is still ?


---
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 #91: [ROCKETMQ-174]Fix spelling errors

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/91
  
Agreed. Any other opinion for this PR? @lizhanhui please resolve the 
conflicts, we could merge this PR later~ 


---
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 #101: [ROCKETMQ-194] log appender support

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/101#discussion_r115471708
  
--- Diff: logappender/README.md ---
@@ -0,0 +1,64 @@
+# RocketMQ-LogAppender   [![Build 
Status](https://travis-ci.org/rocketmq/rocketmq-logappender.svg?branch=master)](https://travis-ci.org/rocketmq/rocketmq-logappender)
 [![Coverage 
Status](https://coveralls.io/repos/github/rocketmq/rocketmq-logappender/badge.svg?branch=master)](https://coveralls.io/github/rocketmq/rocketmq-logappender?branch=master)
+
+
+## Introduction
+RocketMQ-LogAppender is a logging component implement of log4j,log4j2 and 
logback.Taking Apache RocketMQ as broker.
+All logs loged will be send to the topic you define.
+
+## Examples
+
+ log4j example
+
+> config detail
+
+```xml
+
+
+
+
+
+
+
+
+
+
+
+
+
+
--- End diff --

why not use single label as above?


---
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 #101: [ROCKETMQ-194] log appender support

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/101#discussion_r115470775
  
--- Diff: logappender/README.md ---
@@ -0,0 +1,64 @@
+# RocketMQ-LogAppender   [![Build 
Status](https://travis-ci.org/rocketmq/rocketmq-logappender.svg?branch=master)](https://travis-ci.org/rocketmq/rocketmq-logappender)
 [![Coverage 
Status](https://coveralls.io/repos/github/rocketmq/rocketmq-logappender/badge.svg?branch=master)](https://coveralls.io/github/rocketmq/rocketmq-logappender?branch=master)
--- End diff --

Could we remove it if  we want it into parent modue, we can reuse rocektmq 
existing infra.


---
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 #101: [ROCKETMQ-194] log appender support

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/101#discussion_r115472199
  
--- Diff: logappender/pom.xml ---
@@ -0,0 +1,122 @@
+
+
+http://maven.apache.org/POM/4.0.0; 
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+
+org.apache.rocketmq
+rocketmq-all
+4.1.0-incubating-SNAPSHOT
+
+4.0.0
+rocketmq-logappender
+   jar
+rocketmq-logappender ${project.version}
+
+
+UTF-8
+false
+true
+1.7
+1.7
+
+
+
+
+
+org.slf4j
+slf4j-api
+1.7.7
+true
+
+
+
+log4j
+log4j
+1.2.17
+true
+
+
+
+org.apache.logging.log4j
+log4j-core
+2.7
+true
+
+
+
+
+ch.qos.logback
+logback-core
+1.1.2
--- End diff --

The latest version ?


---
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 #101: [ROCKETMQ-194] log appender support

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/101#discussion_r115472312
  
--- Diff: logappender/pom.xml ---
@@ -0,0 +1,122 @@
+
+
+http://maven.apache.org/POM/4.0.0; 
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+
+org.apache.rocketmq
+rocketmq-all
+4.1.0-incubating-SNAPSHOT
+
+4.0.0
+rocketmq-logappender
+   jar
+rocketmq-logappender ${project.version}
+
+
+UTF-8
+false
+true
+1.7
+1.7
+
+
+
+
+
+org.slf4j
+slf4j-api
+1.7.7
+true
--- End diff --

We must consider the suitable dependency here.


---
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 #101: [ROCKETMQ-194] log appender support

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/101#discussion_r115473097
  
--- Diff: style/rmq_checkstyle.xml ---
@@ -30,6 +30,12 @@
 
 
 
+
+
+
+
--- End diff --

Why change this rule~


---
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 #101: [ROCKETMQ-194] log appender support

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/101#discussion_r115472704
  
--- Diff: 
logappender/src/main/java/org/apache/rocketmq/logappender/log4j/RocketmqLog4jAppender.java
 ---
@@ -0,0 +1,193 @@
+/*
+ * 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.logappender.log4j;
+
+import org.apache.rocketmq.common.message.Message;
+import org.apache.rocketmq.logappender.common.ProducerInstance;
+import org.apache.log4j.AppenderSkeleton;
+import org.apache.log4j.helpers.LogLog;
+import org.apache.log4j.spi.ErrorCode;
+import org.apache.log4j.spi.LoggingEvent;
+import org.apache.rocketmq.client.producer.MQProducer;
+
+/**
+ * Log4j Appender Component
+ */
+public class RocketmqLog4jAppender extends AppenderSkeleton {
+
+/**
+ * appended message tag define
--- End diff --

Capitalize the first letter~


---
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 #101: [ROCKETMQ-194] log appender support

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/101#discussion_r115471296
  
--- Diff: logappender/README.md ---
@@ -0,0 +1,64 @@
+# RocketMQ-LogAppender   [![Build 
Status](https://travis-ci.org/rocketmq/rocketmq-logappender.svg?branch=master)](https://travis-ci.org/rocketmq/rocketmq-logappender)
 [![Coverage 
Status](https://coveralls.io/repos/github/rocketmq/rocketmq-logappender/badge.svg?branch=master)](https://coveralls.io/github/rocketmq/rocketmq-logappender?branch=master)
+
+
+## Introduction
+RocketMQ-LogAppender is a logging component implement of log4j,log4j2 and 
logback.Taking Apache RocketMQ as broker.
--- End diff --

What is "implement of " It's confused. IMO, this project is used to send 
log message using RocektMQ. while log4j, log4j2 and logback is just a  adapter 
for SL4J


---
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 #99: Correct comment information

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/99
  
@liumian97 if you correct the PR info as @lindzh pointed out, may be a good 
start for this PR merging:-)


---
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 #100: BugFix: ROCKETMQ-191

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/100
  
That's a good polish, but I must point out the 2 point for this 
optimization. 1. please comply with PR best practice for title as you have 
corrected before. 2. you can consider the order when many logic and operator 
together, such as 0&&1&&1(similar 1||0||0, nor 0||0||1), making your logic more 
effective. For this context, linux platform and netty epoll judge, which one 
happen to 0 more easily.


---
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 #96: [ROCKETMQ-187]Measure the code coverage for In...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/96
  
LGTM


---
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.
---

Re: Website improvements to help drive community

[vongosling]

Hi,

We have finished the second round wiki polish. Thanks bruce advice :-). We
added more articles about the following topics:

1. More examples(Simple Message Example，Order Message Example，Broadcast
Example, Schedule Message Example, Batch Message Example)
2. Architecture
3. Deployment(Configuration, CLI and HA parts)
4. Best Practice


Honestly hope every guys to help review. If you have any questions at all,
please feel free to ask in the maillist or request a PR~


2017-03-20 17:22 GMT+08:00 Von Gosling :

> Hi Bruce,
>
> Thanks for your suggestion :-)
>
> We will start the doc improvements in the community, hoping to finish them
> before RocketMQ’s next release. Besides what you have said parts, we hope
> to add some extra works for Apache RocketMQ wiki, including:
>
> 1. Example（Overview, Simple Message Example，Order Message
> Example，Broadcast Example, Schedule Message Example, Batch Message Example)
> 2. Architecture
> 3. Deployment(Configuration, CLI and HA parts)
> 4. Best Practice
> 5. Ecosystem Integration
>
>
> Hi, guys ~ Are you interested in the improvement tasks ? please let me
> know :-)
>
> > 在 2017年3月19日，01:11，Bruce Snyder  写道：
> >
> > Today someone emailed me directly about their interest in becoming a
> > committer to the RocketMQ project. I explained that committership is
> based
> > on the merit of their contributions to the project and I encouraged them
> to
> > join the dev@rocket mailing list to start some discussions. Upon looking
> > for the link to the page on the RocketMQ site that provides the mailing
> > lists, it was very difficult to locate. So I have some suggestions for
> > improving the website to make it more user-friendly and to drive more
> > community contribution. Below are a some of my suggestions for starters:
> >
> > * The content from the Contributing page that currently resides on the
> old
> > Github project's wiki (
> > https://github.com/apache/incubator-rocketmq/blob/master/CONTRIBUTING.md
> ).
> > This content should be moved to the Community page on the new ASF site.
> >
> > * From the new Community page, please link to the Best Practice in Pull
> > Request page.
> >
> > * The title of the Contact page (
> > http://rocketmq.incubator.apache.org/about/contact/) should be changed
> to
> > 'Support' or 'Mailing Lists' and made much more prominent instead of
> buried
> > with the Team page. Please also consider adding a link to it in the
> header
> > where you see Documentation, Blog, Community, etc.
> >
> > * Place a link to the Download page in the header where you see
> > Documentation, Blog, Community, etc.
> >
> > * You should consider moving the Blog to the home page to show more
> project
> > activity to anyone who visits the site.
> >
> > * The Branching Model page outlines the use of Git Flow and yet it
> provides
> > no reference to the official Git Flow documentation or scripts. Please
> add
> > some attribution to the original docs from Vincent Driessen (
> > http://nvie.com/posts/a-successful-git-branching-model/) and the
> git-flow
> > tools (https://github.com/nvie/gitflow).
> >
> > Last, but definitely not least, please always keep in mind when writing
> > documentation that you are writing for people who are learning RocketMQ.
> > Such new users need to understand the RocketMQ architecture and how to
> use
> > it in a production environment. Currently, there is very little
> > documentation available that addresses using RocketMQ in a production
> > environment. I'm sure that there is knowledge of such operational usage
> of
> > RocketMQ from it's deep use at Alibaba. I would really like to see that
> > knowledge documented to begin to prove to new users that RocketMQ is
> > production ready and already used on probably the biggest e-commerce
> sites
> > in the world.
> >
> > Please always keep the following question in mind as you write new
> features
> > and documentation: If I were a new user of RocketMQ with zero knowledge,
> do
> > the docs provide enough information to develop with and deploy RocketMQ
> for
> > production use?
> >
> > Bruce
> >
> > --
> > perl -e 'print
> > unpack("u30","D0G)U8V4\@4VYY9&5R\"F)R=6-E+G-N>61E >
> > ActiveMQ in Action: http://bit.ly/2je6cQ
> > Blog: http://bsnyder.org/ 
> > Twitter: http://twitter.com/brucesnyder
>
>


-- 
Nothing is impossible

[GitHub] incubator-rocketmq issue #94: [ROCKETMQ-179] Fix errors of test cases

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/94
  
Great~


---
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 #82: [ROCKETMQ-121]Support message filtering based ...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/82
  
please @lizhanhui @shroman help to review this great 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 #82: [ROCKETMQ-121]Support message filtering based ...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/82
  
I think only servtool‘s module dependency, so there is no transitive 
dependency pollution on sdk, right?


---
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 #90: [ROCKETMQ-172]log improvement for rocketmq cli...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/90
  
+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 #90: [ROCKETMQ-172]log improvement for rocke...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/90#discussion_r111316295
  
--- Diff: 
client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java ---
@@ -577,12 +577,12 @@ public void operationComplete(ResponseFuture 
responseFuture) {
 }
 } else {
 if (!responseFuture.isSendRequestOK()) {
-pullCallback.onException(new 
MQClientException("send request failed", responseFuture.getCause()));
--- End diff --

Please replace {} with +



---
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 #77: [ROCKETMQ-140]Guard MQVesion methods.

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/77
  
Sorry to response for this PR. It seems good ~


---
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 #87: [ROCKETMQ-161] Update runbroker.sh and runserv...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/87
  
@shroman @lizhanhui Could you help us to review this long-time delayed 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 #91: [ROCKETMQ-174]Fix spelling errors

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/91
  
@shroman @zhouxinyu Could you help review 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-externals issue #7: Rename README.txt to README.md; Add w...

[vongosling]

Github user vongosling commented on the issue:

https://github.com/apache/incubator-rocketmq-externals/pull/7
  
@netroby i think you are familiar with rocketmq PR process. Could you 
modify your PR topic and make association with Jira isssue?Now ,you can not 
point up the component~


---
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 #82: [ROCKETMQ-121]Support message filtering...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/82#discussion_r108885505
  
--- Diff: conf/logback_broker.xml ---
@@ -222,6 +222,29 @@
 
 
 
+
+${user.home}/logs/rocketmqlogs/filter.log
+true
+
+
${user.home}/logs/rocketmqlogs/otherdays/filter.%i.log
--- End diff --

otherdays？


---
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 #82: [ROCKETMQ-121]Support message filtering...

[vongosling]

Github user vongosling commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/82#discussion_r108887571
  
--- Diff: 
filter/src/main/java/org/apache/rocketmq/filter/util/BloomFilter.java ---
@@ -0,0 +1,332 @@
+/*
+ * 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.filter.util;
+
+import java.util.Arrays;
+
+/**
+ * Simple implement of bloom filter.
+ */
+public class BloomFilter {
--- End diff --

Recommended existent mature BloomFilter class


---
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.
---