Rocketmq: getOffsetInQueueByTime return the max offset when time greater than the storetime of last message

[xxd763795151]

The issue tracker is ONLY used for bug report(feature request need to follow RIP process). Keep in mind, please check whether there is an existing same report before your raise a new one.

Alternately (especially if your communication is not a bug report), you can send mail to our mailing lists. We welcome any friendly suggestions, bug fixes, collaboration and other improvements.

Please ensure that your bug report is clear and that it is complete. Otherwise, we may be unable to understand it or to reproduce it, either of which would prevent us from fixing the bug. We strongly recommend the report(bug report or feature request) could include some hints as the following:

BUG REPORT

1. Please describe the issue you observed:

• What did you do (The steps to reproduce)?

method: org.apache.rocketmq.store.ConsumeQueue#getOffsetInQueueByTime

public long getOffsetInQueueByTime(final long timestamp) 

when the param "time" is greater than the storage time of the last message，it should return the max offset of this queue instead of the offset of the second-to-last message(an unexpected value).

• What did you expect to see?

• What did you see instead?

1. Please tell us about your environment:

2. Other information (e.g. detailed explanation, logs, related issues, suggestions how to fix, etc):

[panzhi33]

This is expected。If you get the last position 5, it will happen BufferUnderflowException。Only when the second to last position is 4, then the last message is obtained

[RongtongJin]

I will close the issue first but plz feel free to reopen it if you have other questions.

[xxd763795151]

This is expected。If you get the last position 5, it will happen BufferUnderflowException。Only when the second to last position is 4, then the last message is obtained

@panzhi33 这个不是会不会出现BufferUnderflowException的问题。是消息偏移的获取问题。如果第1条消息的offset是1，那么第5条消息的offset是5，而不应当是4。
当然了，如果按我们编程习惯来说，索引应当是0开始。但不能套用到这里的offset上，因为如果第1条消息的offset是1的话，那第0条消息的offset就0了。
这个地方计算的问题，连带了很多的副作用了。比如根据时间戳重置消息偏移的时候，重置到当前时间的时候 ，重置的永远是当前最大偏移-1，而其它时间点都是正常的。同样在代码实现上根据2分查找获取偏移值的时候，除了最后一条消息，获取方式都是离时间戳最近的那个，结果预期，但是时间 大于等于当前时间的时候，就只能获取到倒数第2条。

[xxd763795151]

如果第1条消息offset是0，那第0条消息呢。

[panzhi33]

如果第1条消息offset是0，那第0条消息呢。

不好意思，上面图的点位标的有点问题，图在画细一点。可以把consumeQueue每条消息的结构画进去。消息的起始物理偏移量physical offset(long 8字节)+消息大小size(int 4字节)+tagsCode(long 8字节)，每条数据的大小为20个字节。使用byteBuffer来存储。
如果是第一条消息，那么它的起始position就是0x20，byteBuffer.getLong()就可以获取到第一条消息的物理位置。那么第二条消息的起始position就是1x20，以此类推。如果按照你那样来算，第5条消息是5的话，起始position就是5x20，再通过byteBuffer.getLong()来获取消息的物理位置，就会发生BufferUnderflowException异常了。

重置消费位点确实会发生你说的那个问题，只能重置到倒数第二条。但是mq已经提供了解决办法，如果药重置全部的消息，时间戳哪里设置-1，就不会走getOffsetInQueueByTime这个方法了。

如果你想到解决办法，也可以提pr，一起学习

[xxd763795151]

如果第1条消息offset是0，那第0条消息呢。

不好意思，上面图的点位标的有点问题，图在画细一点。可以把consumeQueue每条消息的结构画进去。消息的起始物理偏移量physical offset(long 8字节)+消息大小size(int 4字节)+tagsCode(long 8字节)，每条数据的大小为20个字节。使用byteBuffer来存储。
如果是第一条消息，那么它的起始position就是0x20，byteBuffer.getLong()就可以获取到第一条消息的物理位置。那么第二条消息的起始position就是1x20，以此类推。如果按照你那样来算，第5条消息是5的话，起始position就是5x20，再通过byteBuffer.getLong()来获取消息的物理位置，就会发生BufferUnderflowException异常了。

重置消费位点确实会发生你说的那个问题，只能重置到倒数第二条。但是mq已经提供了解决办法，如果药重置全部的消息，时间戳哪里设置-1，就不会走getOffsetInQueueByTime这个方法了。

如果你想到解决办法，也可以提pr，一起学习

@panzhi33 是的，我之前尝试提交个pr，修复这个问题，但是感觉风险太大，最终没有提交本地代码，也把这个pr关了。
当时的思路是这样的，在最后返回的时候加上一个CQ_STORE_UNIT_SIZE，保证每次返回的时候，当前查到的消息的偏移上加上一。因为如果没有消息的话，直接就返回0了，不会走到这一步。如果走到这个地方，就会返回我们平常说的消息位点的这个实际值。

return (mappedFile.getFileFromOffset() + offset + CQ_STORE_UNIT_SIZE) / CQ_STORE_UNIT_SIZE;

然后将几处涉及到根据这个偏移来获取物理position的地方进行修改，就是将查询的offset再减1。改完之后，我发现，改动的地方有点多，我根本无法确认所有修改的地方都没有问题，也没时间进行测试，风险太高，就没提交最终代码，并把pr也关了。
也无法猜测最初作者是怎么思考的，这个地方计算的offset的为什么不是实际消息的位点，所以就把这个issue留了下来。