Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] rocksdbcq batch dispatch should be removed #8698

Closed
3 tasks done
LetLetMe opened this issue Sep 14, 2024 · 1 comment · Fixed by #8739
Closed
3 tasks done

[Bug] rocksdbcq batch dispatch should be removed #8698

LetLetMe opened this issue Sep 14, 2024 · 1 comment · Fixed by #8739

Comments

@LetLetMe
Copy link
Contributor

LetLetMe commented Sep 14, 2024

Before Creating the Bug Report

  • I found a bug, not just asking a question, which should be created in GitHub Discussions.

  • I have searched the GitHub Issues and GitHub Discussions of this repository and believe that this is not a duplicate.

  • I have confirmed that this bug belongs to the current repository, not other repositories of RocketMQ.

Runtime platform environment

all

RocketMQ version

all

JDK Version

all

Describe the Bug

RocksDBConsumeQueueStore 中在处理dispatchRequest时候会有一个攒批的动作,这里的初衷是为了提高效率,但是实际上并不会起到什么作用,因为rocksdb首先写入的memtable,这是纯内存操作,不会有多少性能损耗。而且这里还有导致一个bug,就是消息比较少的情况,攒批可能会比较久,这样会导致消息消费延迟,严重的话会导致最后15条消息一直消费不到。

In RocksDBConsumeQueueStore, there is a batching action when handling dispatchRequest. The original intention here is to improve efficiency, but in reality, it doesn't have much effect because RocksDB first writes to the memtable, which is a pure memory operation and doesn't incur much performance overhead. Additionally, this can lead to a bug where, in cases of fewer messages, batching might take a long time, resulting in message consumption delays. In severe cases, the last 15 messages might never be consumed.

Steps to Reproduce

对于攒批我们一般的处理方式是设置超时时间,当时间超过最大超市时间则强制刷新,但是putMessagePositionInfoWrapper没有专门的线程去定时刷新,而是由dispatchRequest触发的,当没有消息的时候,设置超时时间也没有用,所以我们在这里应该直接将这个batch去掉

For batching, our usual approach is to set a timeout, forcing a flush when the time exceeds the maximum timeout. However, putMessagePositionInfoWrapper does not have a dedicated thread for periodic flushing; instead, it is triggered by dispatchRequest. When there are no messages, setting a timeout is ineffective. Therefore, we should remove the batch directly in this case.

What Did You Expect to See?

nothing

What Did You See Instead?

nothing

Additional Context

nothing

@RongtongJin
Copy link
Contributor

image
我发现不存在该问题,因为doReput中最终会调用finishCommitLogDispatch方法

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants