Re: The new branching model has been put into effect.

2017-08-01 Thread Xinyu Zhou
Don't worry. The latest release is right, contains everything in the release 
note. We just should do a duplex merge after the release, simple, right?



On Wed, Aug 02, 2017 at 8:51am, Roman Shtykh < rsht...@yahoo.com.invalid 
[rsht...@yahoo.com.invalid] > wrote:
Hi Yukon,
Thanks a lot!
There are two issues then.
1. The latest release is wrong and probably doesn't contain everything as 
advertised. Shall we cancel it and re-release?Do mentors consider it a problem?

2. How can we make sure that the next release is done correctly?
Obviously, 'develop' branch introduces extra efforts for merging and release. 
It doesn't seem work well.
-- Roman



On Tuesday, August 1, 2017 6:02 PM, yukon  wrote:


Hi Roman,

The last release didn't do a right merge between develop and master branch,
so we saw a big commit gap.
I have made a synchronization between these two branches just now.

Regards

On Tue, Aug 1, 2017 at 4:12 PM, Roman Shtykh 
wrote:

> Hi Yukon,
> Can you confirm the release was done correctly (with all modifications
> incorporated, as advertised)?
> -- Roman
>
>
> On Friday, July 14, 2017 10:37 AM, yukon  wrote:
>
>
> Hi,
>
>
> Thanks guys, actually the release manager will do a sync between `master`
> and `develop`(duplex merge) when release a new version, so this diff only
> will occur in the gap between two releases.
>
> But... Maybe there is something wrong, I will check it~
>
>
> Regards,
> yukon
>
> On Thu, Jul 13, 2017 at 10:00 AM, Zhanhui Li  wrote:
>
> > Agree, develop and master branches should not split. We need to fix this
> > issue.
> >
> > > 在 2017年7月12日,下午9:23,Roman Shtykh  写道:
> > >
> > > Folks,
> > > Please have a look at https://github.com/apache/
> incubator-rocketmq/tree/
> > develop
> > > This branch is 82 commits ahead, 74 commits behind master. <-- how can
> > this happen? considering we had a recent release.
> > > Do we have the situation Justin described before -- "In project where
> we
> > had this split it just ends up being more work, people accidentally check
> > into the wrong branch and I’ve see a lot of merge issues"?
> > >
> > > I'll say it again, but it is very inconvenient to use gitflow model on
> > github (when on github you normally commit on master branch).
> > >
> > > -- Roman
> > >
> > > On Friday, March 17, 2017 11:41 AM, Justin Mclean <
> > jus...@classsoftware.com> wrote:
> > >
> > >
> > > Hi,
> > >
> > > I also don't see there's a real need to split master and develop. In
> > project where we had this split it just ends up being more work, people
> > accidentally check into the wrong branch and I’ve see a lot of merge
> issues.
> > >
> > > That being said the project is free to select whatever model they want.
> > >
> > > Thanks,
> > > Justin
> > >
> >
> >
>
>
>

[GitHub] incubator-rocketmq pull request #134: Rito - #44 - Extracting out duplicated...

2017-08-01 Thread shroman
Github user shroman commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/134#discussion_r130764893
  
--- Diff: 
client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java ---
@@ -1215,9 +1210,13 @@ public TopicRouteData 
getTopicRouteInfoFromNameServer(final String topic, final
 assert response != null;
 switch (response.getCode()) {
 case ResponseCode.TOPIC_NOT_EXIST: {
-if (!topic.equals(MixAll.DEFAULT_TOPIC))
+if (allowTopicNotExist && 
!topic.equals(MixAll.DEFAULT_TOPIC)) {
 log.warn("get Topic [{}] RouteInfoFromNameServer is 
not exist value", topic);
-break;
+break;
--- End diff --

well, I mean you can have one for `case ResponseCode.TOPIC_NOT_EXIST`, 
don't you?


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

2017-08-01 Thread shroman
Github user shroman commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/134#discussion_r130764799
  
--- Diff: 
client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java ---
@@ -1215,9 +1210,13 @@ public TopicRouteData 
getTopicRouteInfoFromNameServer(final String topic, final
 assert response != null;
 switch (response.getCode()) {
 case ResponseCode.TOPIC_NOT_EXIST: {
-if (!topic.equals(MixAll.DEFAULT_TOPIC))
+if (allowTopicNotExist && 
!topic.equals(MixAll.DEFAULT_TOPIC)) {
 log.warn("get Topic [{}] RouteInfoFromNameServer is 
not exist value", topic);
-break;
+break;
+} else {
+// TODO LOG
--- End diff --

ok then


---
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: The new branching model has been put into effect.

2017-08-01 Thread Roman Shtykh
Hi Yukon,
Thanks a lot!
There are two issues then.
1. The latest release is wrong and probably doesn't contain everything as 
advertised. Shall we cancel it and re-release?Do mentors consider it a problem?

2. How can we make sure that the next release is done correctly?
Obviously, 'develop' branch introduces extra efforts for merging and release. 
It doesn't seem work well.
-- Roman



On Tuesday, August 1, 2017 6:02 PM, yukon  wrote:
 

 Hi Roman,

The last release didn't do a right merge between develop and master branch,
so we saw a big commit gap.
I have made a synchronization between these two branches just now.

Regards

On Tue, Aug 1, 2017 at 4:12 PM, Roman Shtykh 
wrote:

> Hi Yukon,
> Can you confirm the release was done correctly (with all modifications
> incorporated, as advertised)?
> -- Roman
>
>
>    On Friday, July 14, 2017 10:37 AM, yukon  wrote:
>
>
>  Hi,
>
>
> Thanks guys, actually the release manager will do a sync between `master`
> and `develop`(duplex merge) when release a new version, so this diff only
> will occur in the gap between two releases.
>
> But... Maybe there is something wrong, I will check it~
>
>
> Regards,
> yukon
>
> On Thu, Jul 13, 2017 at 10:00 AM, Zhanhui Li  wrote:
>
> > Agree, develop and master branches should not split. We need to fix this
> > issue.
> >
> > > 在 2017年7月12日,下午9:23,Roman Shtykh  写道:
> > >
> > > Folks,
> > > Please have a look at https://github.com/apache/
> incubator-rocketmq/tree/
> > develop
> > > This branch is 82 commits ahead, 74 commits behind master. <-- how can
> > this happen? considering we had a recent release.
> > > Do we have the situation Justin described before -- "In project where
> we
> > had this split it just ends up being more work, people accidentally check
> > into the wrong branch and I’ve see a lot of merge issues"?
> > >
> > > I'll say it again, but it is very inconvenient to use gitflow model on
> > github (when on github you normally commit on master branch).
> > >
> > > -- Roman
> > >
> > >    On Friday, March 17, 2017 11:41 AM, Justin Mclean <
> > jus...@classsoftware.com> wrote:
> > >
> > >
> > > Hi,
> > >
> > > I also don't see there's a real need to split master and develop. In
> > project where we had this split it just ends up being more work, people
> > accidentally check into the wrong branch and I’ve see a lot of merge
> issues.
> > >
> > > That being said the project is free to select whatever model they want.
> > >
> > > Thanks,
> > > Justin
> > >
> >
> >
>
>
>

   

[GitHub] incubator-rocketmq issue #134: Rito - #44 - Extracting out duplicated code f...

2017-08-01 Thread coveralls
Github user coveralls commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/134
  

[![Coverage 
Status](https://coveralls.io/builds/12638697/badge)](https://coveralls.io/builds/12638697)

Coverage decreased (-0.3%) to 38.79% when pulling 
**28e14be3ea8fbe24814bcdebd61b36d94dd5b3b2 on Ritabrata-TW:master** into 
**d4149207e27ed3516f1f06407b55986790b806ae on apache:master**.



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

2017-08-01 Thread coveralls
Github user coveralls commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/134
  

[![Coverage 
Status](https://coveralls.io/builds/12638697/badge)](https://coveralls.io/builds/12638697)

Coverage decreased (-0.3%) to 38.79% when pulling 
**28e14be3ea8fbe24814bcdebd61b36d94dd5b3b2 on Ritabrata-TW:master** into 
**d4149207e27ed3516f1f06407b55986790b806ae on apache:master**.



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

2017-08-01 Thread coveralls
Github user coveralls commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/134
  

[![Coverage 
Status](https://coveralls.io/builds/12638697/badge)](https://coveralls.io/builds/12638697)

Coverage decreased (-0.3%) to 38.79% when pulling 
**28e14be3ea8fbe24814bcdebd61b36d94dd5b3b2 on Ritabrata-TW:master** into 
**d4149207e27ed3516f1f06407b55986790b806ae on apache:master**.



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

2017-08-01 Thread Ritabrata-TW
Github user Ritabrata-TW commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/134#discussion_r130596160
  
--- Diff: 
store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java ---
@@ -717,22 +718,28 @@ public long getEarliestMessageTime(String topic, int 
queueId) {
 long minLogicOffset = logicQueue.getMinLogicOffset();
 
 SelectMappedBufferResult result = 
logicQueue.getIndexBuffer(minLogicOffset / ConsumeQueue.CQ_STORE_UNIT_SIZE);
-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 e) {
-} finally {
-result.release();
-}
-}
+Long storeTime = getStoreTime(result);
+return storeTime;
--- End diff --

Yup. Made the 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 pull request #134: Rito - #44 - Extracting out duplicated...

2017-08-01 Thread Ritabrata-TW
Github user Ritabrata-TW commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/134#discussion_r130595893
  
--- Diff: 
client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java ---
@@ -1215,9 +1210,13 @@ public TopicRouteData 
getTopicRouteInfoFromNameServer(final String topic, final
 assert response != null;
 switch (response.getCode()) {
 case ResponseCode.TOPIC_NOT_EXIST: {
-if (!topic.equals(MixAll.DEFAULT_TOPIC))
+if (allowTopicNotExist && 
!topic.equals(MixAll.DEFAULT_TOPIC)) {
 log.warn("get Topic [{}] RouteInfoFromNameServer is 
not exist value", topic);
-break;
+break;
+} else {
+// TODO LOG
--- End diff --

@shroman The TODO was already there. Not sure what to do about it right 
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 #134: Rito - #44 - Extracting out duplicated...

2017-08-01 Thread Ritabrata-TW
Github user Ritabrata-TW commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/134#discussion_r130595647
  
--- Diff: 
client/src/main/java/org/apache/rocketmq/client/impl/MQClientAPIImpl.java ---
@@ -1215,9 +1210,13 @@ public TopicRouteData 
getTopicRouteInfoFromNameServer(final String topic, final
 assert response != null;
 switch (response.getCode()) {
 case ResponseCode.TOPIC_NOT_EXIST: {
-if (!topic.equals(MixAll.DEFAULT_TOPIC))
+if (allowTopicNotExist && 
!topic.equals(MixAll.DEFAULT_TOPIC)) {
 log.warn("get Topic [{}] RouteInfoFromNameServer is 
not exist value", topic);
-break;
+break;
--- End diff --

@shroman We don't have two breaks. The part you are have referred to shows 
the diff where the break from 1220 has been removed and the break on 1215 has 
been added. 


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

2017-08-01 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/incubator-rocketmq/pull/58


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

2017-08-01 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/incubator-rocketmq/pull/124


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

2017-08-01 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/incubator-rocketmq/pull/125


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

2017-08-01 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/incubator-rocketmq/pull/71


---
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 #138: add implement SelectMessageQueueByConsistentH...

2017-08-01 Thread zhouxinyu
Github user zhouxinyu commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/138
  
Hi, @Jaskey could you please have a look at 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 #120: [ROCKETMQ-224] add rocketmq client log...

2017-08-01 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/incubator-rocketmq/pull/120


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

2017-08-01 Thread coveralls
Github user coveralls commented on the issue:

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

[![Coverage 
Status](https://coveralls.io/builds/12636404/badge)](https://coveralls.io/builds/12636404)

Coverage increased (+0.9%) to 39.575% when pulling 
**a5d73d080e49e63457c2be4b996c339d6e488fae on 
lindzh:fix_consumer_pull_msg_size** into 
**9bb6eae4bd35697808174a2ff9195e393450ec91 on apache:master**.



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

2017-08-01 Thread coveralls
Github user coveralls commented on the issue:

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

[![Coverage 
Status](https://coveralls.io/builds/12636199/badge)](https://coveralls.io/builds/12636199)

Coverage decreased (-0.4%) to 38.718% when pulling 
**6cdfaacb2e01eae477d561c8f6efd0fed1afce09 on lindzh:logger_appender_test** 
into **d4149207e27ed3516f1f06407b55986790b806ae on apache:master**.



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

2017-08-01 Thread lindzh
Github user lindzh commented on a diff in the pull request:

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

Yes,this problem has been fixed to maxMsgNums <= messageTotal


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

2017-08-01 Thread vsair
Github user vsair commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r130556323
  
--- Diff: 
client/src/main/java/org/apache/rocketmq/client/log/ClientLogger.java ---
@@ -99,7 +111,12 @@ private static Logger createLogger(final String 
loggerName) {
 }
 
 public static Logger getLog() {
-return log;
+if (log == null) {
--- End diff --

why not modify the init method?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


Re: The new branching model has been put into effect.

2017-08-01 Thread yukon
Hi Roman,

The last release didn't do a right merge between develop and master branch,
so we saw a big commit gap.
I have made a synchronization between these two branches just now.

Regards

On Tue, Aug 1, 2017 at 4:12 PM, Roman Shtykh 
wrote:

> Hi Yukon,
> Can you confirm the release was done correctly (with all modifications
> incorporated, as advertised)?
> -- Roman
>
>
> On Friday, July 14, 2017 10:37 AM, yukon  wrote:
>
>
>  Hi,
>
>
> Thanks guys, actually the release manager will do a sync between `master`
> and `develop`(duplex merge) when release a new version, so this diff only
> will occur in the gap between two releases.
>
> But... Maybe there is something wrong, I will check it~
>
>
> Regards,
> yukon
>
> On Thu, Jul 13, 2017 at 10:00 AM, Zhanhui Li  wrote:
>
> > Agree, develop and master branches should not split. We need to fix this
> > issue.
> >
> > > 在 2017年7月12日,下午9:23,Roman Shtykh  写道:
> > >
> > > Folks,
> > > Please have a look at https://github.com/apache/
> incubator-rocketmq/tree/
> > develop
> > > This branch is 82 commits ahead, 74 commits behind master. <-- how can
> > this happen? considering we had a recent release.
> > > Do we have the situation Justin described before -- "In project where
> we
> > had this split it just ends up being more work, people accidentally check
> > into the wrong branch and I’ve see a lot of merge issues"?
> > >
> > > I'll say it again, but it is very inconvenient to use gitflow model on
> > github (when on github you normally commit on master branch).
> > >
> > > -- Roman
> > >
> > >On Friday, March 17, 2017 11:41 AM, Justin Mclean <
> > jus...@classsoftware.com> wrote:
> > >
> > >
> > > Hi,
> > >
> > > I also don't see there's a real need to split master and develop. In
> > project where we had this split it just ends up being more work, people
> > accidentally check into the wrong branch and I’ve see a lot of merge
> issues.
> > >
> > > That being said the project is free to select whatever model they want.
> > >
> > > Thanks,
> > > Justin
> > >
> >
> >
>
>
>


[GitHub] incubator-rocketmq issue #120: [ROCKETMQ-224] add rocketmq client log4j2 log...

2017-08-01 Thread coveralls
Github user coveralls commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/120
  

[![Coverage 
Status](https://coveralls.io/builds/12635815/badge)](https://coveralls.io/builds/12635815)

Coverage increased (+0.2%) to 38.771% when pulling 
**0ea6994712784d700aa365c2e121306b898b3355 on lindzh:fix_client_logger** into 
**0c5e53db6f4d0ed9f25747379a8b679e2da5392d on apache:master**.



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

2017-08-01 Thread coveralls
Github user coveralls commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/120
  

[![Coverage 
Status](https://coveralls.io/builds/12635815/badge)](https://coveralls.io/builds/12635815)

Coverage increased (+0.2%) to 38.771% when pulling 
**0ea6994712784d700aa365c2e121306b898b3355 on lindzh:fix_client_logger** into 
**0c5e53db6f4d0ed9f25747379a8b679e2da5392d on apache:master**.



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

2017-08-01 Thread coveralls
Github user coveralls commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/120
  

[![Coverage 
Status](https://coveralls.io/builds/12635815/badge)](https://coveralls.io/builds/12635815)

Coverage increased (+0.2%) to 38.771% when pulling 
**0ea6994712784d700aa365c2e121306b898b3355 on lindzh:fix_client_logger** into 
**0c5e53db6f4d0ed9f25747379a8b679e2da5392d on apache:master**.



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

2017-08-01 Thread lindzh
Github user lindzh commented on a diff in the pull request:

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

This comment has been deleted.


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

2017-08-01 Thread 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...

2017-08-01 Thread 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.
---


Re: The new branching model has been put into effect.

2017-08-01 Thread Roman Shtykh
Hi Yukon,
Can you confirm the release was done correctly (with all modifications 
incorporated, as advertised)?
-- Roman


On Friday, July 14, 2017 10:37 AM, yukon  wrote:
 

 Hi,


Thanks guys, actually the release manager will do a sync between `master`
and `develop`(duplex merge) when release a new version, so this diff only
will occur in the gap between two releases.

But... Maybe there is something wrong, I will check it~


Regards,
yukon

On Thu, Jul 13, 2017 at 10:00 AM, Zhanhui Li  wrote:

> Agree, develop and master branches should not split. We need to fix this
> issue.
>
> > 在 2017年7月12日,下午9:23,Roman Shtykh  写道:
> >
> > Folks,
> > Please have a look at https://github.com/apache/incubator-rocketmq/tree/
> develop
> > This branch is 82 commits ahead, 74 commits behind master. <-- how can
> this happen? considering we had a recent release.
> > Do we have the situation Justin described before -- "In project where we
> had this split it just ends up being more work, people accidentally check
> into the wrong branch and I’ve see a lot of merge issues"?
> >
> > I'll say it again, but it is very inconvenient to use gitflow model on
> github (when on github you normally commit on master branch).
> >
> > -- Roman
> >
> >    On Friday, March 17, 2017 11:41 AM, Justin Mclean <
> jus...@classsoftware.com> wrote:
> >
> >
> > Hi,
> >
> > I also don't see there's a real need to split master and develop. In
> project where we had this split it just ends up being more work, people
> accidentally check into the wrong branch and I’ve see a lot of merge issues.
> >
> > That being said the project is free to select whatever model they want.
> >
> > Thanks,
> > Justin
> >
>
>

   

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

2017-08-01 Thread lindzh
Github user lindzh commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/120#discussion_r130544050
  
--- Diff: 
client/src/main/java/org/apache/rocketmq/client/log/ClientLogger.java ---
@@ -27,16 +28,19 @@
 public static final String CLIENT_LOG_ROOT = "rocketmq.client.logRoot";
 public static final String CLIENT_LOG_MAXINDEX = 
"rocketmq.client.logFileMaxIndex";
 public static final String CLIENT_LOG_LEVEL = 
"rocketmq.client.logLevel";
+
 private static Logger log;
 
-static {
-log = createLogger(LoggerName.CLIENT_LOGGER_NAME);
+public static Class logClass = null;
--- End diff --

Fixed it with reflect


---
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 #137: Develop

2017-08-01 Thread coveralls
Github user coveralls commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/137
  

[![Coverage 
Status](https://coveralls.io/builds/12635031/badge)](https://coveralls.io/builds/12635031)

Coverage decreased (-0.03%) to 39.09% when pulling 
**9bb6eae4bd35697808174a2ff9195e393450ec91 on develop** into 
**d4149207e27ed3516f1f06407b55986790b806ae on master**.



---
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 #137: Develop

2017-08-01 Thread coveralls
Github user coveralls commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/137
  

[![Coverage 
Status](https://coveralls.io/builds/12635031/badge)](https://coveralls.io/builds/12635031)

Coverage decreased (-0.03%) to 39.09% when pulling 
**9bb6eae4bd35697808174a2ff9195e393450ec91 on develop** into 
**d4149207e27ed3516f1f06407b55986790b806ae on master**.



---
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 #137: Develop

2017-08-01 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/incubator-rocketmq/pull/137


---
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 #127: [ROCKETMQ-233] Fix pull interval issue

2017-08-01 Thread shroman
Github user shroman commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/127
  
@lizhanhui Got it, thanks! How about adding comments with explanations for 
each policy class? I think developers will appreciate that.


---
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 #127: [ROCKETMQ-233] Fix pull interval issue

2017-08-01 Thread shroman
Github user shroman commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/127#discussion_r130530039
  
--- Diff: 
client/src/main/java/org/apache/rocketmq/client/consumer/PullIntervalPolicy.java
 ---
@@ -0,0 +1,25 @@
+/*
+ * 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.client.consumer;
+
+public interface PullIntervalPolicy {
+
+void update(PullStatus status);
+
+int getPullInterval();
--- End diff --

What about making it `long`? _delay_ of executePullRequestLater(...) is 
long.


---
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 #139: [ROCKETMQ-242] mqclient can not fetch ...

2017-08-01 Thread shroman
Github user shroman commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/139#discussion_r130528221
  
--- Diff: 
client/src/main/java/org/apache/rocketmq/client/impl/factory/MQClientInstance.java
 ---
@@ -255,19 +255,17 @@ public void start() throws MQClientException {
 }
 
 private void startScheduledTask() {
-if (null == this.clientConfig.getNamesrvAddr()) {
--- End diff --

I think what @qqeasonchen says makes sense, but we have discuss what 
behavior we want to have.

To me, if addresses have been manually specified,
it makes sense to fetch name servers only if a call to manually specified 
servers fail. No need to do periodic calls. What do you think?


---
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 #139: [ROCKETMQ-242] mqclient can not fetch ...

2017-08-01 Thread qqeasonchen
Github user qqeasonchen commented on a diff in the pull request:

https://github.com/apache/incubator-rocketmq/pull/139#discussion_r130527608
  
--- Diff: 
client/src/main/java/org/apache/rocketmq/client/impl/factory/MQClientInstance.java
 ---
@@ -255,19 +255,17 @@ public void start() throws MQClientException {
 }
 
 private void startScheduledTask() {
-if (null == this.clientConfig.getNamesrvAddr()) {
--- End diff --

in fact, even though we specified the web endpoint, client will not fetch 
name server addresses any more beacase of below code in start():

`// If not specified,looking address from name server
if (null == this.clientConfig.getNamesrvAddr()) {
this.mQClientAPIImpl.fetchNameServerAddr();
}`


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

2017-08-01 Thread shroman
Github user shroman commented on the issue:

https://github.com/apache/incubator-rocketmq/pull/134
  
@Ritabrata-TW In other words, your pull request description has to be in 
this format
```
[ROCKETMQ-xxx] Message describing the issue.

Link to ROCKETMQ-xxx, which is the JIRA issue.
```


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

2017-08-01 Thread 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...

2017-08-01 Thread 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...

2017-08-01 Thread 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.
---