You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by lizhanhui <gi...@git.apache.org> on 2017/06/26 06:05:17 UTC
[GitHub] incubator-rocketmq pull request #127: [ROCKETMQ-233] Fix pull interval issue
GitHub user lizhanhui opened a pull request:
https://github.com/apache/incubator-rocketmq/pull/127
[ROCKETMQ-233] Fix pull interval issue
Make pullInterval effective when pullResult is NO_NEW_MESSAGE or NO_MATCHED_MESSAGE
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/lizhanhui/incubator-rocketmq ROCKETMQ-233
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-rocketmq/pull/127.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #127
----
commit ee38268ce5476b4d5d7c7ee76b56a180d9c8e019
Author: Li Zhanhui <li...@gmail.com>
Date: 2017-06-26T06:02:58Z
Make pullInterval effective when pullResult is NO_NEW_MESSAGE or NO_MATCHED_MESSAGE
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #127: [ROCKETMQ-233] Fix pull interval issue
Posted by zhouxinyu <gi...@git.apache.org>.
Github user zhouxinyu commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/127#discussion_r124161533
--- Diff: test/src/test/java/org/apache/rocketmq/test/client/producer/tls/TlsIntegrationTestBase.java ---
@@ -0,0 +1,29 @@
+/*
+ * 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.producer.tls;
+
+import org.junit.BeforeClass;
+
+public class TlsIntegrationTestBase {
--- End diff --
Any relation between Tls and pull interval?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #127: [ROCKETMQ-233] Fix pull interval issue
Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/127
Looks good, but can you please briefly outline what the problem was -- ROCKETMQ-233 does not say much.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #127: [ROCKETMQ-233] Fix pull interval issue
Posted by lizhanhui <gi...@git.apache.org>.
Github user lizhanhui commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/127#discussion_r124255531
--- Diff: test/src/test/java/org/apache/rocketmq/test/client/producer/tls/TlsIntegrationTestBase.java ---
@@ -0,0 +1,29 @@
+/*
+ * 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.producer.tls;
+
+import org.junit.BeforeClass;
+
+public class TlsIntegrationTestBase {
--- End diff --
No, this file should have been excluded.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #127: [ROCKETMQ-233] Fix pull interval issue
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/127
[](https://:/builds/12127616)
Coverage decreased (-0.02%) to 38.421% when pulling **ee38268ce5476b4d5d7c7ee76b56a180d9c8e019 on lizhanhui:ROCKETMQ-233** into **34b2b81aee127b07e8beb6c8a3c38748ffe26cfd on apache:develop**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #127: [ROCKETMQ-233] Fix pull interval issue
Posted by Jaskey <gi...@git.apache.org>.
Github user Jaskey commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/127#discussion_r124174720
--- Diff: client/src/main/java/org/apache/rocketmq/client/impl/consumer/DefaultMQPushConsumerImpl.java ---
@@ -273,6 +273,17 @@ public void pullMessage(final PullRequest pullRequest) {
final long beginTimestamp = System.currentTimeMillis();
PullCallback pullCallback = new PullCallback() {
+
+ private void scheduleNextPull(PullResult pullResult) {
--- End diff --
what about renaming the method to `correctTagsOffsetAndSchedulePull` since this method is called by the result which needs to correct tags offset before schedule.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #127: [ROCKETMQ-233] Fix pull interval issue
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/127
[](https://coveralls.io/builds/12590821)
Coverage increased (+0.005%) to 38.724% when pulling **d7ccb71c2e97c53d1406f847e8c95d6e25ad0513 on lizhanhui:ROCKETMQ-233** into **118bdec96005ce695295bcf0d9082e0230e69bf7 on apache:develop**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #127: [ROCKETMQ-233] Fix pull interval issue
Posted by lizhanhui <gi...@git.apache.org>.
Github user lizhanhui commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/127
@shroman This issue is to reduce fruitless polling for consumer clients. If a consumer subscribes topics which ship messages no that frequent, it may stop for a while in a smart way before performing the next round of polling, as may relieve some pressure from brokers.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #127: [ROCKETMQ-233] Fix pull interval issue
Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on a diff in the pull request:
https://github.com/apache/incubator-rocketmq/pull/127#discussion_r126346177
--- Diff: client/src/main/java/org/apache/rocketmq/client/impl/consumer/DefaultMQPushConsumerImpl.java ---
@@ -273,6 +273,17 @@ public void pullMessage(final PullRequest pullRequest) {
final long beginTimestamp = System.currentTimeMillis();
PullCallback pullCallback = new PullCallback() {
+
+ private void scheduleNextPull(PullResult pullResult) {
--- End diff --
Let's have it in a comment for the 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #127: [ROCKETMQ-233] Fix pull interval issue
Posted by shroman <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #127: [ROCKETMQ-233] Fix pull interval issue
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/127
[](https://coveralls.io/builds/12592396)
Coverage increased (+0.2%) to 38.906% when pulling **9b082648b80f489b70fda7e4472619fa1adbe81d on lizhanhui:ROCKETMQ-233** into **118bdec96005ce695295bcf0d9082e0230e69bf7 on apache:develop**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq pull request #127: [ROCKETMQ-233] Fix pull interval issue
Posted by shroman <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---