You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "tensory2022 (via GitHub)" <gi...@apache.org> on 2023/10/20 06:18:18 UTC
[I] Code comments of updateConsumeOffset() should be corrected [rocketmq]
tensory2022 opened a new issue, #7489:
URL: https://github.com/apache/rocketmq/issues/7489
### Search before creation
- [X] I had searched in the [issues](https://github.com/apache/rocketmq/issues) and found no similar issues.
### Documentation Related
[org/apache/rocketmq/example/simple/PullConsumer.java]
https://github.com/apache/rocketmq/blob/develop/example/src/main/java/org/apache/rocketmq/example/simple/PullConsumer.java
consumer.updateConsumeOffset() uses the function of updateOffset() in OffsetStore interface here:
[org/apache/rocketmq/client/consumer/store/OffsetStore.java](https://github.com/apache/rocketmq/blob/develop/client/src/main/java/org/apache/rocketmq/client/consumer/store/OffsetStore.java)
In example, the comments show "//update offset to broker" and the interface class code shows "Update the offset,store it in memory".
Or one of the two comments might be wrong. Please correct me if I misunderstood them.
### Are you willing to submit PR?
- [X] Yes I am willing to submit a PR!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Code comments of updateConsumeOffset() should be corrected [rocketmq]
Posted by "tensory2022 (via GitHub)" <gi...@apache.org>.
tensory2022 commented on issue #7489:
URL: https://github.com/apache/rocketmq/issues/7489#issuecomment-1774618414
@joeCarf Thanks for your PR. Hope it will be merged soon.
The code you require is at: org.apache.rocketmq.client.impl.factory.MQClientInstance.
And the scheduled time interval should be 5 seconds instead of 10 seconds I declared before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Code comments of updateConsumeOffset() should be corrected [rocketmq]
Posted by "tensory2022 (via GitHub)" <gi...@apache.org>.
tensory2022 commented on issue #7489:
URL: https://github.com/apache/rocketmq/issues/7489#issuecomment-1772543186
> <img alt="截屏2023-10-20 17 07 41" width="865" src="https://user-images.githubusercontent.com/52153761/276881360-b801a8e5-773b-472d-8304-1327ff800e52.png"> updateOffset这个方法有两个实现，一个是在本地存储offset，一个是将offset存在broker里。这两种实现分别对应着广播消费和集群消费模式，因此我理解这里的comment有些模糊，但是大意应该没有问题
Thanks for your reply :)
There is no difference in the implementations of method updateOffset() between LocalFileOffsetStore and RemoteBrokerOffsetStore. The codes of updateOffset() in RemoteBrokerOffsetStore (Clustering mode as you said) are as follows：
As per the code, I can not found the interaction between client and broker. Therefore, I still think the comment should be corrected. You are right with the fact that the scheduled service would update offset from client memory to broker disk. However, that action is not directly to update offset to broker.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Code comments of updateConsumeOffset() should be corrected [rocketmq]
Posted by "joeCarf (via GitHub)" <gi...@apache.org>.
joeCarf commented on issue #7489:
URL: https://github.com/apache/rocketmq/issues/7489#issuecomment-1772554676
图中的代码只更新了内存中的offset。在`RemoteBrokerOffsetStore`中，有一`persistAll`函数，会将整个offsetTable更新到对应的broker
<img width="926" alt="截屏2023-10-20 19 17 46" src="https://github.com/apache/rocketmq/assets/52153761/87751575-c2e3-4eba-bc2a-7ec1c14ab9d0">
而`LocalFileOffsetStore`中只会写到本地文件中
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Code comments of updateConsumeOffset() should be corrected [rocketmq]
Posted by "joeCarf (via GitHub)" <gi...@apache.org>.
joeCarf commented on issue #7489:
URL: https://github.com/apache/rocketmq/issues/7489#issuecomment-1772364020
<img width="865" alt="截屏2023-10-20 17 07 41" src="https://github.com/apache/rocketmq/assets/52153761/b801a8e5-773b-472d-8304-1327ff800e52">
updateOffset这个方法有两个实现，一个是在本地存储offset，一个是将offset存在broker里。这两种实现分别对应着广播消费和集群消费模式，因此我理解这里的comment有些模糊，但是大意应该没有问题
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Code comments of updateConsumeOffset() should be corrected [rocketmq]
Posted by "tensory2022 (via GitHub)" <gi...@apache.org>.
tensory2022 commented on issue #7489:
URL: https://github.com/apache/rocketmq/issues/7489#issuecomment-1774373252
@joeCarf
I never doubt the offset will be updated to broker, what I care is whether it is done by the method updateConsumeOffset(). If not, the code comment should be corrected.
In additon, you might need to think over this point: "而LocalFileOffsetStore中只会写到本地文件中" . Whether you use LocalFileOffsetStore or RemoteBrokerOffsetStore, the offset will be synchronized to broker by a scheduled service which runs every 10 seconds.
I hope you understand what I'm saying, thank you~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Code comments of updateConsumeOffset() should be corrected [rocketmq]
Posted by "joeCarf (via GitHub)" <gi...@apache.org>.
joeCarf commented on issue #7489:
URL: https://github.com/apache/rocketmq/issues/7489#issuecomment-1774433611
plz take a look at this pr https://github.com/apache/rocketmq/pull/7490
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Code comments of updateConsumeOffset() should be corrected [rocketmq]
Posted by "joeCarf (via GitHub)" <gi...@apache.org>.
joeCarf commented on issue #7489:
URL: https://github.com/apache/rocketmq/issues/7489#issuecomment-1774426203
i get your point, and i will submit a pr to enhance it. btw, i didn't find the `scheduled service which runs every 10 seconds` in LocalFileOffsetStore, will you plz show me the code location
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Code comments of updateConsumeOffset() should be corrected [rocketmq]
Posted by "joeCarf (via GitHub)" <gi...@apache.org>.
joeCarf commented on issue #7489:
URL: https://github.com/apache/rocketmq/issues/7489#issuecomment-1772555145
@tensory2022
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Code comments of updateConsumeOffset() should be corrected [rocketmq]
Posted by "RongtongJin (via GitHub)" <gi...@apache.org>.
RongtongJin closed issue #7489: Code comments of updateConsumeOffset() should be corrected
URL: https://github.com/apache/rocketmq/issues/7489
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org