Skip to content

Commit b14acd3

Browse files
committed
MB-58555: Use fractionOf for calculation of mutation_mem_ratio threshold
...and refactor out the existing percentOf from a couple of places. By casting the integer to a double, we avoid performing the operation with float, which has insufficient precision. We do the same for memory watermarks already. Change-Id: Ia3aab14f24b6d85b3d298c31ca25095e44831452 Reviewed-on: https://review.couchbase.org/c/kv_engine/+/196802 Tested-by: Vesko Karaganev <[email protected]> Reviewed-by: Pavlos Georgiou <[email protected]>
1 parent 8869f92 commit b14acd3

File tree

5 files changed

+47
-23
lines changed

5 files changed

+47
-23
lines changed

engines/ep/src/ep_engine.cc

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
#include <statistics/prometheus.h>
8181
#include <utilities/engine_errc_2_mcbp.h>
8282
#include <utilities/logtags.h>
83+
#include <utilities/math_utilities.h>
8384
#include <xattr/utils.h>
8485
#include <functional>
8586

@@ -98,10 +99,6 @@
9899
using cb::tracing::Code;
99100
using namespace std::string_view_literals;
100101

101-
static size_t percentOf(size_t val, double percent) {
102-
return static_cast<size_t>(static_cast<double>(val) * percent);
103-
}
104-
105102
struct EPHandleReleaser {
106103
void operator()(const EventuallyPersistentEngine*) {
107104
ObjectRegistry::onSwitchThread(nullptr);
@@ -6947,8 +6944,10 @@ void EventuallyPersistentEngine::setMaxDataSize(size_t size) {
69476944
}
69486945

69496946
void EventuallyPersistentEngine::configureMemWatermarksForQuota(size_t quota) {
6950-
configuration.setMemLowWat(percentOf(quota, stats.mem_low_wat_percent));
6951-
configuration.setMemHighWat(percentOf(quota, stats.mem_high_wat_percent));
6947+
configuration.setMemLowWat(
6948+
cb::fractionOf(quota, stats.mem_low_wat_percent));
6949+
configuration.setMemHighWat(
6950+
cb::fractionOf(quota, stats.mem_high_wat_percent));
69526951
}
69536952

69546953
void EventuallyPersistentEngine::configureStorageMemoryForQuota(size_t quota) {

engines/ep/src/memory_tracker.cc

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,13 @@
1313
#include "ep_engine.h"
1414
#include "ep_engine_group.h"
1515
#include "kv_bucket.h"
16+
#include <utilities/math_utilities.h>
1617

1718
bool StrictQuotaMemoryTracker::isBelowMutationMemoryQuota(
1819
size_t pendingBytes) const {
1920
return engine.getEpStats().getEstimatedTotalMemoryUsed() + pendingBytes <
20-
engine.getEpStats().getMaxDataSize() *
21-
engine.getKVBucket()->getMutationMemRatio();
21+
cb::fractionOf(engine.getEpStats().getMaxDataSize(),
22+
engine.getKVBucket()->getMutationMemRatio());
2223
}
2324

2425
bool StrictQuotaMemoryTracker::isBelowMemoryQuota(size_t pendingBytes) const {
@@ -28,8 +29,8 @@ bool StrictQuotaMemoryTracker::isBelowMemoryQuota(size_t pendingBytes) const {
2829

2930
bool StrictQuotaMemoryTracker::isBelowBackfillThreshold() const {
3031
return engine.getEpStats().getEstimatedTotalMemoryUsed() <
31-
engine.getEpStats().getMaxDataSize() *
32-
engine.getKVBucket()->getBackfillMemoryThreshold();
32+
cb::fractionOf(engine.getEpStats().getMaxDataSize(),
33+
engine.getKVBucket()->getBackfillMemoryThreshold());
3334
}
3435

3536
bool StrictQuotaMemoryTracker::needsToFreeMemory() const {

engines/ep/src/replicationthrottle.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "ep_engine.h"
1515
#include "kv_bucket.h"
1616
#include "stats.h"
17+
#include <utilities/math_utilities.h>
1718

1819
ReplicationThrottleEP::ReplicationThrottleEP(
1920
const EventuallyPersistentEngine& engine)
@@ -29,7 +30,7 @@ bool ReplicationThrottleEP::hasSomeMemory() const {
2930
const auto* bucket = engine.getKVBucket();
3031
Expects(bucket);
3132

32-
return memoryUsed <= maxSize * bucket->getMutationMemRatio();
33+
return memoryUsed <= cb::fractionOf(maxSize, bucket->getMutationMemRatio());
3334
}
3435

3536
ReplicationThrottleEP::Status ReplicationThrottleEP::getStatus() const {

engines/ep/tests/module_tests/bucket_quota_change_test.cc

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,14 @@
1919
#include "kvstore/kvstore_iface.h"
2020
#include "test_helpers.h"
2121
#include "vbucket.h"
22+
#include <utilities/math_utilities.h>
2223

2324
#include "../mock/mock_synchronous_ep_engine.h"
2425

2526
#ifdef EP_USE_MAGMA
2627
#include "kvstore/magma-kvstore/magma-kvstore_config.h"
2728
#endif
2829

29-
auto percentOf(size_t val, double percent) {
30-
return static_cast<size_t>(static_cast<double>(val) * percent);
31-
}
32-
3330
class BucketQuotaChangeTest : public STParameterizedBucketTest {
3431
public:
3532
void SetUp() override {
@@ -114,14 +111,14 @@ class BucketQuotaChangeTest : public STParameterizedBucketTest {
114111
}
115112

116113
void setLowWatermark(double percentage) {
117-
auto newValue = percentOf(getCurrentBucketQuota(), percentage);
114+
auto newValue = cb::fractionOf(getCurrentBucketQuota(), percentage);
118115
std::string msg;
119116
engine->setFlushParam("mem_low_wat", std::to_string(newValue), msg);
120117
initialMemLowWatPercent = percentage;
121118
}
122119

123120
void setHighWatermark(double percentage) {
124-
auto newValue = percentOf(getCurrentBucketQuota(), percentage);
121+
auto newValue = cb::fractionOf(getCurrentBucketQuota(), percentage);
125122
std::string msg;
126123
engine->setFlushParam("mem_high_wat", std::to_string(newValue), msg);
127124
initialMemHighWatPercent = percentage;
@@ -135,11 +132,11 @@ class BucketQuotaChangeTest : public STParameterizedBucketTest {
135132
void checkWatermarkValues(size_t quotaValue) {
136133
EXPECT_EQ(initialMemLowWatPercent,
137134
engine->getEpStats().mem_low_wat_percent);
138-
EXPECT_EQ(percentOf(quotaValue, initialMemLowWatPercent),
135+
EXPECT_EQ(cb::fractionOf(quotaValue, initialMemLowWatPercent),
139136
engine->getConfiguration().getMemLowWat());
140137
EXPECT_EQ(initialMemHighWatPercent,
141138
engine->getEpStats().mem_high_wat_percent);
142-
EXPECT_EQ(percentOf(quotaValue, initialMemHighWatPercent),
139+
EXPECT_EQ(cb::fractionOf(quotaValue, initialMemHighWatPercent),
143140
engine->getConfiguration().getMemHighWat());
144141
}
145142

@@ -162,8 +159,8 @@ class BucketQuotaChangeTest : public STParameterizedBucketTest {
162159
engine->getKVBucket()->getOneRWUnderlying()->getStat("memory_quota",
163160
magmaQuota);
164161
EXPECT_EQ(
165-
percentOf(quotaValue,
166-
magmaKVStoreConfig.getMagmaMemQuotaRatio()),
162+
cb::fractionOf(quotaValue,
163+
magmaKVStoreConfig.getMagmaMemQuotaRatio()),
167164
magmaQuota * engine->getConfiguration().getMaxNumShards());
168165
}
169166
#endif
@@ -291,10 +288,10 @@ class BucketQuotaChangeTest : public STParameterizedBucketTest {
291288
// backfills, and storage engine quota, before it changes the actual
292289
// quota, check that those values have been updated now (and that quota
293290
// is still the same).
294-
EXPECT_EQ(percentOf(newQuota, initialMemLowWatPercent),
291+
EXPECT_EQ(cb::fractionOf(newQuota, initialMemLowWatPercent),
295292
engine->getConfiguration().getMemLowWat());
296293

297-
EXPECT_EQ(percentOf(newQuota, initialMemHighWatPercent),
294+
EXPECT_EQ(cb::fractionOf(newQuota, initialMemHighWatPercent),
298295
engine->getConfiguration().getMemHighWat());
299296

300297
checkMaxRunningBackfills(newQuota);

utilities/math_utilities.h

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*
2+
* Copyright 2023-Present Couchbase, Inc.
3+
*
4+
* Use of this software is governed by the Business Source License included
5+
* in the file licenses/BSL-Couchbase.txt. As of the Change Date specified
6+
* in that file, in accordance with the Business Source License, use of this
7+
* software will be governed by the Apache License, Version 2.0, included in
8+
* the file licenses/APL2.txt.
9+
*/
10+
#pragma once
11+
12+
namespace cb {
13+
14+
/**
15+
* Calculate a fraction of an integer, using sufficient precision to represent
16+
* the addressable virtual memory.
17+
*
18+
* @param val An integer value
19+
* @param frac The fraction of the value to calculate [0.-1.]
20+
*/
21+
template <typename T>
22+
auto fractionOf(T val, double frac) {
23+
return static_cast<T>(static_cast<double>(val) * frac);
24+
}
25+
26+
} // namespace cb

0 commit comments

Comments
 (0)