Skip to content

Commit 00779bc

Browse files
committed
MB-56917: Don't queue BGItems if above mutation watermark
Prevent queuing items into the BGFetchQueue if the size of the BGFetchItem will increase above the mutation watermark. If the required memory exceeds the mutation watermark eviction will be invoked in order to decrease memory usage. This is achieved through calling memoryCondition(). Change-Id: If5dfd3432345575ee96fee2f110e47d85ab8ffb4 Reviewed-on: https://review.couchbase.org/c/kv_engine/+/201256 Reviewed-by: Jim Walker <[email protected]> Tested-by: Build Bot <[email protected]>
1 parent dde138c commit 00779bc

File tree

4 files changed

+72
-0
lines changed

4 files changed

+72
-0
lines changed

engines/ep/src/ep_engine.cc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5439,6 +5439,14 @@ cb::engine_errc EventuallyPersistentEngine::checkPrivilege(
54395439
return cb::engine_errc::failed;
54405440
}
54415441

5442+
cb::engine_errc EventuallyPersistentEngine::checkMemoryForBGFetch(
5443+
size_t pendingBytes) {
5444+
if (memoryTracker->isBelowMutationMemoryQuota(pendingBytes)) {
5445+
return cb::engine_errc::success;
5446+
}
5447+
return memoryCondition();
5448+
}
5449+
54425450
cb::engine_errc EventuallyPersistentEngine::testPrivilege(
54435451
CookieIface& cookie,
54445452
cb::rbac::Privilege priv,

engines/ep/src/ep_engine.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -969,6 +969,17 @@ class EventuallyPersistentEngine : public EngineIface, public DcpIface {
969969
cb::rbac::Privilege priv,
970970
CollectionID) const;
971971

972+
/**
973+
* Checks if adding the specified memory size will keep memory usage below
974+
* the mutation watermark. If so, returns success; otherwise, invoke
975+
* eviction and return no_memory/temp_failure.
976+
*
977+
* @param pendingBytes The size of memory to be added.
978+
* @return cb::engine_errc::success if addition memory size is available,
979+
* otherwise no_memory / temp_failure.
980+
*/
981+
cb::engine_errc checkMemoryForBGFetch(size_t pendingBytes);
982+
972983
/**
973984
* Test the access for the given privilege for the bucket.scope.collection
974985
* This differs from checkPrivilege in that the error case has no side

engines/ep/src/ep_vb.cc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,14 @@ cb::engine_errc EPVBucket::bgFetch(HashTable::HashBucketLock&& hbl,
829829
// reference if that's the case
830830
// schedule to the current batch of background fetch of the given
831831
// vbucket
832+
833+
size_t requiredMemory =
834+
StoredValue::getRequiredStorage(key) + sizeof(FrontEndBGFetchItem);
835+
auto status = engine.checkMemoryForBGFetch(requiredMemory);
836+
if (status != cb::engine_errc::success) {
837+
return status;
838+
}
839+
832840
const auto filter =
833841
isMeta ? ValueFilter::KEYS_ONLY
834842
: dynamic_cast<EPBucket&>(*bucket)

engines/ep/tests/module_tests/evp_store_test.cc

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -773,6 +773,51 @@ TEST_P(EPBucketFullEvictionTest, BgfetchSucceedsUntilMutationWatermark) {
773773
destroy_mock_cookie(cookie2);
774774
}
775775

776+
TEST_P(EPBucketFullEvictionTest, DontQueueBGFetchItemAboveMutationWatermark) {
777+
// This test relies on precise memory tracking
778+
const auto& stats = engine->getEpStats();
779+
if (!stats.isMemoryTrackingEnabled()) {
780+
GTEST_SKIP();
781+
}
782+
783+
ASSERT_EQ(cb::engine_errc::success,
784+
store->setVBucketState(vbid, vbucket_state_active, {}));
785+
EXPECT_LT(stats.getPreciseTotalMemoryUsed(), stats.getMaxDataSize());
786+
787+
int valueSize = 1 * 1024 * 1024;
788+
789+
// Load document of size 1MiB
790+
const std::string value(valueSize, 'x');
791+
auto key0 = makeStoredDocKey("key_0");
792+
793+
// store item
794+
store_item(vbid, key0, value);
795+
flush_vbucket_to_disk(vbid);
796+
797+
// evict item
798+
evict_key(vbid, key0);
799+
800+
// try to bgFetch item
801+
auto options = static_cast<get_options_t>(
802+
QUEUE_BG_FETCH | HONOR_STATES | TRACK_REFERENCE | DELETE_TEMP |
803+
HIDE_LOCKED_CAS | TRACK_STATISTICS | GET_DELETED_VALUE);
804+
auto cookie = create_mock_cookie();
805+
806+
// set bucket quota using current memory usage as 93%
807+
double mutationWat = engine->getConfiguration().getMutationMemRatio();
808+
engine->setMaxDataSize(stats.getPreciseTotalMemoryUsed() / mutationWat);
809+
810+
auto gv = store->get(key0, vbid, cookie, options);
811+
EXPECT_NE(cb::engine_errc::would_block, gv.getStatus());
812+
813+
// BGFetch queue should remain empty
814+
auto vb = store->getVBucket(vbid);
815+
EXPECT_FALSE(vb->hasPendingBGFetchItems());
816+
EXPECT_LE(stats.getPreciseTotalMemoryUsed(), stats.getMaxDataSize());
817+
818+
destroy_mock_cookie(cookie);
819+
}
820+
776821
TEST_P(EPBucketFullEvictionTest, ExpiryFindNonResidentItem) {
777822
EXPECT_EQ(cb::engine_errc::success,
778823
store->setVBucketState(vbid, vbucket_state_active, {}));

0 commit comments

Comments
 (0)