From 6d42a955129da0c9d43cd63f77550e1f465045f6 Mon Sep 17 00:00:00 2001 From: Ryoji Kurosawa Date: Mon, 16 Dec 2024 15:36:04 +0900 Subject: [PATCH] add limit for content_scan api --- include/sharksfin/api.h | 5 ++- memory/src/Iterator.h | 5 ++- memory/src/api.cpp | 6 ++- memory/src/api_helper.cpp | 3 +- memory/src/api_helper.h | 4 +- shirakami/src/Iterator.cpp | 7 ++-- shirakami/src/Iterator.h | 3 ++ shirakami/src/Storage.cpp | 3 +- shirakami/src/Storage.h | 2 + shirakami/src/api.cpp | 3 +- shirakami/test/ApiTest.cpp | 79 ++++++++++++++++++++++++++++++++++++++ 11 files changed, 108 insertions(+), 12 deletions(-) diff --git a/include/sharksfin/api.h b/include/sharksfin/api.h index 47c514c..6b5a7d4 100644 --- a/include/sharksfin/api.h +++ b/include/sharksfin/api.h @@ -788,6 +788,7 @@ inline std::ostream& operator<<(std::ostream& out, EndPointKind value) { * @param end_key the content key of ending position * @param end_kind end-point kind of the ending position * @param result [OUT] an iterator handle over the key range + * @param limit the max number of entries to be fetched. 0 indicates no limit. * @param reverse whether or not the iterator scans in reverse order (from end to begin) * Current limitation on reverse = true is that end_kind must be EndPointKind::UNBOUND and at most one entry is * fetched as the scan result @@ -801,7 +802,9 @@ extern "C" StatusCode content_scan( StorageHandle storage, Slice begin_key, EndPointKind begin_kind, Slice end_key, EndPointKind end_kind, - IteratorHandle* result, bool reverse = false); + IteratorHandle* result, + std::size_t limit = 0, + bool reverse = false); /** * @brief advances the given iterator. diff --git a/memory/src/Iterator.h b/memory/src/Iterator.h index 005dc42..51bef52 100644 --- a/memory/src/Iterator.h +++ b/memory/src/Iterator.h @@ -46,17 +46,19 @@ class Iterator { * @param begin_kind end-point kind of the beginning position * @param end_key the content key of ending position * @param end_kind end-point kind of the ending position + * @param limit the max number of entries to be fetched. 0 indicates no limit. * @param reverse whether or not the iterator scans in reverse order (from end to begin) */ Iterator( Storage* owner, Slice begin_key, EndPointKind begin_kind, - Slice end_key, EndPointKind end_kind, bool reverse = false) + Slice end_key, EndPointKind end_kind, std::size_t limit = 0, bool reverse = false) : owner_(owner) , next_key_(begin_kind == EndPointKind::UNBOUND ? std::string_view {} : begin_key.to_string_view()) , end_key_(end_kind == EndPointKind::UNBOUND ? Slice {} : end_key) , end_type_(interpret_end_kind(end_kind)) , state_(interpret_begin_kind(begin_kind)) + , limit_(limit) , reverse_(reverse) {} @@ -107,6 +109,7 @@ class Iterator { Buffer end_key_; End end_type_; State state_; + std::size_t limit_; bool reverse_; Slice payload_ {}; diff --git a/memory/src/api.cpp b/memory/src/api.cpp index 864c649..b275a78 100644 --- a/memory/src/api.cpp +++ b/memory/src/api.cpp @@ -399,16 +399,18 @@ StatusCode content_scan( Slice begin_key, EndPointKind begin_kind, Slice end_key, EndPointKind end_kind, IteratorHandle* result, + std::size_t limit, bool reverse) { log_entry << fn_name << " transaction:" << transaction << " storage:" << storage << binstring(begin_key) << " begin_kind:" << begin_kind << - binstring(end_key) << " end_kind:" << end_kind; + binstring(end_key) << " end_kind:" << end_kind << + " limit:" << limit << " reverse:" << reverse; auto rc = impl::content_scan( transaction, storage, begin_key, begin_kind, end_key, end_kind, - result, reverse); + result, limit, reverse); log_rc(rc, fn_name); log_exit << fn_name << " rc:" << rc << " result:" << *result; return rc; diff --git a/memory/src/api_helper.cpp b/memory/src/api_helper.cpp index 1ab54e1..ec890c7 100644 --- a/memory/src/api_helper.cpp +++ b/memory/src/api_helper.cpp @@ -505,6 +505,7 @@ StatusCode content_scan( Slice begin_key, EndPointKind begin_kind, Slice end_key, EndPointKind end_kind, IteratorHandle* result, + std::size_t limit, bool reverse) { auto tx = unwrap(transaction); auto st = unwrap(storage); @@ -514,7 +515,7 @@ StatusCode content_scan( auto iterator = std::make_unique( st, begin_key, begin_kind, - end_key, end_kind, reverse); + end_key, end_kind, limit, reverse); *result = wrap(iterator.release()); return StatusCode::OK; } diff --git a/memory/src/api_helper.h b/memory/src/api_helper.h index 4ee3f34..fbc4a03 100644 --- a/memory/src/api_helper.h +++ b/memory/src/api_helper.h @@ -179,7 +179,9 @@ StatusCode content_scan( StorageHandle storage, Slice begin_key, EndPointKind begin_kind, Slice end_key, EndPointKind end_kind, - IteratorHandle* result, bool reverse); + IteratorHandle* result, + std::size_t limit, + bool reverse); StatusCode iterator_next(IteratorHandle handle); diff --git a/shirakami/src/Iterator.cpp b/shirakami/src/Iterator.cpp index a4f8a82..e3fef15 100644 --- a/shirakami/src/Iterator.cpp +++ b/shirakami/src/Iterator.cpp @@ -36,6 +36,7 @@ Iterator::Iterator( EndPointKind begin_kind, Slice end_key, EndPointKind end_kind, + std::size_t limit, bool reverse) : owner_(owner), state_(State::INIT), @@ -44,6 +45,7 @@ Iterator::Iterator( begin_kind_(begin_kind), end_key_(end_kind == EndPointKind::UNBOUND ? std::string_view{} : end_key.to_string_view()), end_kind_(end_kind), + limit_(limit), reverse_(reverse) {} Iterator::~Iterator() { @@ -201,13 +203,10 @@ StatusCode Iterator::open_cursor() { break; } - // reverse scan can fetch only one entry for now - std::size_t max_size = reverse_ ? 1 : 0; - auto res = api::open_scan(tx_->native_handle(), owner_->handle(), begin_key_, begin_endpoint, - end_key_, end_endpoint, handle_, max_size, reverse_); + end_key_, end_endpoint, handle_, limit_, reverse_); tx_->last_call_status(res); correct_transaction_state(*tx_, res); if(res == Status::WARN_NOT_FOUND) { diff --git a/shirakami/src/Iterator.h b/shirakami/src/Iterator.h index 3d051d5..b055128 100644 --- a/shirakami/src/Iterator.h +++ b/shirakami/src/Iterator.h @@ -46,6 +46,7 @@ class Iterator { * @param end_key the content key of ending position * @param end_kind whether or not ending position is exclusive * If end_key is not empty and end kind UNBOUND, the end_kind is reduced to PREFIXED_INCLUSIVE + * @param limit the max number of entries to be fetched. 0 indicates no limit. * @param reverse whether or not the iterator scans in reverse order (from end to begin) */ Iterator( // NOLINT(performance-unnecessary-value-param) @@ -53,6 +54,7 @@ class Iterator { Transaction* tx, Slice begin_key, EndPointKind begin_kind, Slice end_key, EndPointKind end_kind, + std::size_t limit = 0, bool reverse = false ); @@ -99,6 +101,7 @@ class Iterator { EndPointKind begin_kind_{}; std::string end_key_{}; EndPointKind end_kind_{}; + std::size_t limit_{}; bool reverse_{}; bool key_value_readable_{false}; bool need_scan_close_{false}; diff --git a/shirakami/src/Storage.cpp b/shirakami/src/Storage.cpp index 47be1f7..7963a6d 100644 --- a/shirakami/src/Storage.cpp +++ b/shirakami/src/Storage.cpp @@ -66,12 +66,13 @@ StatusCode Storage::scan(Transaction* tx, Slice begin_key, EndPointKind begin_kind, Slice end_key, EndPointKind end_kind, std::unique_ptr& out, + std::size_t limit, bool reverse ) { if(! tx->active()) return StatusCode::ERR_INACTIVE_TRANSACTION; out = std::make_unique(this, tx, begin_key, begin_kind, - end_key, end_kind, reverse); + end_key, end_kind, limit, reverse); return StatusCode::OK; } diff --git a/shirakami/src/Storage.h b/shirakami/src/Storage.h index 3367ffd..83d9226 100644 --- a/shirakami/src/Storage.h +++ b/shirakami/src/Storage.h @@ -105,6 +105,7 @@ class Storage { * @param end_key the content key of ending position * @param end_kind end-point kind of the ending position * @param out [OUT] the created iterator + * @param limit the max number of entries to be fetched. 0 indicates no limit. * @param reverse whether or not the scan in reverse order (from end to begin) * @return the operation status */ @@ -112,6 +113,7 @@ class Storage { Slice begin_key, EndPointKind begin_kind, Slice end_key, EndPointKind end_kind, std::unique_ptr& out, + std::size_t limit = 0, bool reverse = false ); diff --git a/shirakami/src/api.cpp b/shirakami/src/api.cpp index f4bf1e4..89e815f 100644 --- a/shirakami/src/api.cpp +++ b/shirakami/src/api.cpp @@ -465,6 +465,7 @@ StatusCode content_scan( Slice begin_key, EndPointKind begin_kind, Slice end_key, EndPointKind end_kind, IteratorHandle* result, + std::size_t limit, bool reverse) { auto tx = unwrap(transaction); if (! tx->active()) return StatusCode::ERR_INACTIVE_TRANSACTION; @@ -474,7 +475,7 @@ StatusCode content_scan( return StatusCode::ERR_INVALID_STATE; } std::unique_ptr iter{}; - auto rc = stg->scan(tx, begin_key, begin_kind, end_key, end_kind, iter, reverse); + auto rc = stg->scan(tx, begin_key, begin_kind, end_key, end_kind, iter, limit, reverse); if(rc != StatusCode::OK) { return rc; } diff --git a/shirakami/test/ApiTest.cpp b/shirakami/test/ApiTest.cpp index 14a693b..2906ffd 100644 --- a/shirakami/test/ApiTest.cpp +++ b/shirakami/test/ApiTest.cpp @@ -937,6 +937,7 @@ TEST_F(ShirakamiApiTest, scan_range) { EXPECT_EQ(transaction_exec(db, {}, &S::test, &s), StatusCode::OK); EXPECT_EQ(database_close(db), StatusCode::OK); } + TEST_F(ShirakamiApiTest, scan) { DatabaseOptions options; options.attribute(KEY_LOCATION, path()); @@ -1026,6 +1027,83 @@ TEST_F(ShirakamiApiTest, scan) { EXPECT_EQ(database_close(db), StatusCode::OK); } +TEST_F(ShirakamiApiTest, scan_with_limit) { + DatabaseOptions options; + options.attribute(KEY_LOCATION, path()); + DatabaseHandle db; + ASSERT_EQ(database_open(options, &db), StatusCode::OK); + HandleHolder dbh { db }; + + struct S { + static TransactionOperation prepare(TransactionHandle tx, void* args) { + auto st = extract(args); + if (content_put(tx, st, "a", "NG") != StatusCode::OK) { + return TransactionOperation::ERROR; + } + if (content_put(tx, st, "a1", "NG") != StatusCode::OK) { + return TransactionOperation::ERROR; + } + if (content_put(tx, st, "b", "B") != StatusCode::OK) { + return TransactionOperation::ERROR; + } + if (content_put(tx, st, "c", "C") != StatusCode::OK) { + return TransactionOperation::ERROR; + } + if (content_put(tx, st, "d", "NG") != StatusCode::OK) { + return TransactionOperation::ERROR; + } + return TransactionOperation::COMMIT; + } + static TransactionOperation test(TransactionHandle tx, void* args) { + auto st = extract(args); + IteratorHandle iter; + if (content_scan( + tx, st, + "b", EndPointKind::PREFIXED_INCLUSIVE, + "", EndPointKind::UNBOUND, + &iter, + 2) != StatusCode::OK) { + return TransactionOperation::ERROR; + } + HandleHolder closer { iter }; + + Slice s; + if (iterator_next(iter) != StatusCode::OK) { + return TransactionOperation::ERROR; + } + if (iterator_get_key(iter, &s) != StatusCode::OK || s != "b") { + return TransactionOperation::ERROR; + } + if (iterator_get_value(iter, &s) != StatusCode::OK || s != "B") { + return TransactionOperation::ERROR; + } + + if (iterator_next(iter) != StatusCode::OK) { + return TransactionOperation::ERROR; + } + if (iterator_get_key(iter, &s) != StatusCode::OK || s != "c") { + return TransactionOperation::ERROR; + } + if (iterator_get_value(iter, &s) != StatusCode::OK || s != "C") { + return TransactionOperation::ERROR; + } + + if (iterator_next(iter) != StatusCode::NOT_FOUND) { + return TransactionOperation::ERROR; + } + return TransactionOperation::COMMIT; + } + StorageHandle st; + }; + S s; + ASSERT_EQ(storage_create(db, "s", &s.st), StatusCode::OK); + HandleHolder sth { s.st }; + + EXPECT_EQ(transaction_exec(db, {}, &S::prepare, &s), StatusCode::OK); + EXPECT_EQ(transaction_exec(db, {}, &S::test, &s), StatusCode::OK); + EXPECT_EQ(database_close(db), StatusCode::OK); +} + TEST_F(ShirakamiApiTest, reverse_scan) { // currently reverse scan can fetch only one entry with unbonded end point DatabaseOptions options; @@ -1056,6 +1134,7 @@ TEST_F(ShirakamiApiTest, reverse_scan) { "a", EndPointKind::PREFIXED_INCLUSIVE, "", EndPointKind::UNBOUND, &iter, + 1, // reverse scan must have limit 1 true // reverse scan ) != StatusCode::OK) { return TransactionOperation::ERROR;