From b9207d233d2583a87b9956c52014b2828a29169b Mon Sep 17 00:00:00 2001 From: Jim Walker Date: Fri, 6 May 2022 14:54:01 +0100 Subject: [PATCH] MB-35297: Fix "RangeScan::setStateIdle invalid state:State::Cancelled" The continue "self-cancelling" code is relying on the cancel being detected in a key/value callback. If the scan has no data, no callback and the code attempts to move from cancel to idle (invalid). Add a check for cancel before calling into KVStore::scan Change-Id: I4ecfd42cd57770883b18b63035b2a5dc500e5696 Reviewed-on: https://review.couchbase.org/c/kv_engine/+/174585 Tested-by: Build Bot Reviewed-by: Trond Norbye --- engines/ep/src/range_scans/range_scan.cc | 9 +++++++++ engines/ep/tests/module_tests/range_scan_test.cc | 14 ++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/engines/ep/src/range_scans/range_scan.cc b/engines/ep/src/range_scans/range_scan.cc index f103889eb1..43ee89e5d9 100644 --- a/engines/ep/src/range_scans/range_scan.cc +++ b/engines/ep/src/range_scans/range_scan.cc @@ -74,6 +74,15 @@ cb::rangescan::Id RangeScan::createScan(const CookieIface& cookie, cb::engine_errc RangeScan::continueScan(KVStoreIface& kvstore) { EP_LOG_DEBUG("RangeScan {} continue for {}", uuid, getVBucketId()); + + // Only attempt scan when !cancelled + if (isCancelled()) { + // ensure the client/cookie sees cancelled + handleStatus(cb::engine_errc::range_scan_cancelled); + // but return success so the scan now gets cleaned-up + return cb::engine_errc::success; + } + auto status = kvstore.scan(*scanCtx); switch (status) { diff --git a/engines/ep/tests/module_tests/range_scan_test.cc b/engines/ep/tests/module_tests/range_scan_test.cc index 4819d4d623..75244fde1c 100644 --- a/engines/ep/tests/module_tests/range_scan_test.cc +++ b/engines/ep/tests/module_tests/range_scan_test.cc @@ -425,6 +425,20 @@ TEST_P(RangeScanTest, create_cancel) { EXPECT_TRUE(handler->scannedItems.empty()); } +TEST_P(RangeScanTest, create_cancel_no_data) { + // this scan will generate no callbacks internally, but must still safely + // cancel + auto uuid = createScan(scanCollection, {"0"}, {"0\xFF"}); + auto vb = store->getVBucket(vbid); + EXPECT_EQ(cb::engine_errc::success, vb->cancelRangeScan(uuid, true)); + runNextTask(*task_executor->getLpTaskQ()[AUXIO_TASK_IDX], + "RangeScanContinueTask"); + + // Nothing read + EXPECT_TRUE(handler->scannedKeys.empty()); + EXPECT_TRUE(handler->scannedItems.empty()); +} + // Check that if a scan has been continue (but is waiting to run), it can be // cancelled. When the task runs the scan cancels. TEST_P(RangeScanTest, create_continue_is_cancelled) {