Skip to content

Commit

Permalink
MB-35297: Evaluate all RangeScan continue limits
Browse files Browse the repository at this point in the history
The current if/else if block is incorrect and for example means that
if both an item + byte limit are enabled, the item limit is the only
one checked, and the scan may not end at the expected point.

The updated if/else if now checks all limits for each call.

Change-Id: Ib606c164aa9060f459ffba75ca4741d94fb3f71c
Reviewed-on: https://review.couchbase.org/c/kv_engine/+/179901
Tested-by: Jim Walker <jim@couchbase.com>
Reviewed-by: Paolo Cocchi <paolo.cocchi@couchbase.com>
  • Loading branch information
jimwwalker committed Sep 14, 2022
1 parent 7debb15 commit 87a72fd
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 8 deletions.
19 changes: 11 additions & 8 deletions engines/ep/src/range_scans/range_scan.cc
Original file line number Diff line number Diff line change
Expand Up @@ -537,14 +537,17 @@ void RangeScan::incrementValueFromDisk() {
}

bool RangeScan::areLimitsExceeded() const {
if (continueRunState.cState.limits.itemLimit) {
return continueRunState.itemCount >=
continueRunState.cState.limits.itemLimit;
} else if (continueRunState.cState.limits.timeLimit.count()) {
return now() >= continueRunState.scanContinueDeadline;
} else if (continueRunState.cState.limits.byteLimit) {
return continueRunState.byteCount >=
continueRunState.cState.limits.byteLimit;
if (continueRunState.cState.limits.itemLimit &&
continueRunState.itemCount >=
continueRunState.cState.limits.itemLimit) {
return true;
} else if (continueRunState.cState.limits.timeLimit.count() &&
now() >= continueRunState.scanContinueDeadline) {
return true;
} else if (continueRunState.cState.limits.byteLimit &&
continueRunState.byteCount >=
continueRunState.cState.limits.byteLimit) {
return true;
}
return false;
}
Expand Down
13 changes: 13 additions & 0 deletions engines/ep/tests/module_tests/range_scan_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,19 @@ TEST_P(RangeScanTest, user_prefix_with_item_limit_2) {
expectedKeys.size() / 2);
}

// Test has a smaller byte-limit which must take affect before the item count
TEST_P(RangeScanTest, user_prefix_with_two_limits) {
auto expectedKeys = getUserKeys();
testRangeScan(expectedKeys,
scanCollection,
{"user"},
{"user\xFF"},
2, // 2 items
std::chrono::milliseconds(0),
1, // 1 byte
expectedKeys.size());
}

TEST_P(RangeScanTest, user_prefix_with_time_limit) {
// Replace time with a function that ticks per call, forcing the scan to
// yield for every item
Expand Down

0 comments on commit 87a72fd

Please sign in to comment.