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

[ISSUE #7961] use BoundaryType in binarySearchInCQByTime #7968

Merged

Conversation

Koado
Copy link
Contributor

@Koado Koado commented Mar 26, 2024

Which Issue(s) This PR Fixes

[Enhancement] use BoundaryType in binarySearchInCQByTime #7961

Brief Description

use BoundaryType in binarySearchInCQByTime instead of absolute value comparison.

When performing a range query, pass BoundaryType.LOWER to get the left boundary msg offset, pass BoundaryType.UPPER to get the right boundary msg offset

How Did You Test This Change?

I wrote a test case, the timestamp 1711351134289 does not match an exact offset, when we call mqAdminExt.searchLowerBoundaryOffset(), it returns the offset whose storeTime is the smallest storeTime that is greater than the target storeTime. When we call mqAdminExt.searchUpperBoundaryOffset(), it returns the offset whose storeTime is the biggest storeTime that is smaller than the target storeTime.

@Test
    public void searchOffsetTest() throws MQBrokerException, RemotingException, InterruptedException, MQClientException {
        TopicRouteData routeData = mqAdminExt.examineTopicRouteInfo("TopicTest");
        TopicStatsTable topicStatsTable = mqAdminExt.examineTopicStats("TopicTest");
        Map<MessageQueue, TopicOffset> queueMap = topicStatsTable.getOffsetTable();
        for (Map.Entry<MessageQueue, TopicOffset> entry : queueMap.entrySet()) {
            MessageQueue mq = entry.getKey();
            long offset_1 = mqAdminExt.searchLowerBoundaryOffset(mq, 1711351134289L);
            long offset_2 = mqAdminExt.searchUpperBoundaryOffset(mq, 1711351134289L);
            Assert.assertTrue(offset_1 != offset_2);
            Assert.assertTrue(offset_1 == offset_2 + 1);
        }
    }

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 37.93103% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 42.85%. Comparing base (dde4fcc) to head (c1672a6).
Report is 1 commits behind head on develop.

Files Patch % Lines
...rocketmq/store/queue/RocksDBConsumeQueueTable.java 35.71% 15 Missing and 3 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #7968      +/-   ##
=============================================
+ Coverage      42.83%   42.85%   +0.02%     
- Complexity     10347    10352       +5     
=============================================
  Files           1270     1270              
  Lines          88617    88636      +19     
  Branches       11395    11398       +3     
=============================================
+ Hits           37957    37983      +26     
+ Misses         45976    45962      -14     
- Partials        4684     4691       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RongtongJin RongtongJin merged commit fb6f9e4 into apache:develop Mar 28, 2024
9 of 10 checks passed
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 this pull request may close these issues.

4 participants