Skip to content

Commit c4ede32

Browse files
jimwwalkerdaverigby
authored andcommitted
MB-37643: Set expiry relative to uptime
The calculation of document expiry when max_ttl is > 30 days didn't include the uptime of memcached. Once memcached had an uptime > 30days the generated expiry was in the past. Change-Id: Iee39808830e8a995ddc888dea8435d44cee0da7d Reviewed-on: http://review.couchbase.org/121202 Well-Formed: Build Bot <[email protected]> Reviewed-by: Dave Rigby <[email protected]> Tested-by: Build Bot <[email protected]>
1 parent 51f35ef commit c4ede32

File tree

3 files changed

+41
-8
lines changed

3 files changed

+41
-8
lines changed

engines/ep/src/ep_engine.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2236,7 +2236,7 @@ EventuallyPersistentEngine::getExpiryParameters(rel_time_t exptime) const {
22362236
// it makes sense as an expiry time.
22372237
if (exptime == 0) {
22382238
if (limit.count() > (60 * 60 * 24 * 30)) {
2239-
exptime = ep_abs_time(limit.count());
2239+
exptime = ep_abs_time(ep_current_time() + limit.count());
22402240
} else {
22412241
exptime = limit.count();
22422242
}

engines/ep/tests/module_tests/kv_bucket_test.cc

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1381,18 +1381,18 @@ TEST_F(StoreIfTest, store_if_basic) {
13811381
EXPECT_EQ(cb::engine_errc::predicate_failed, rv.status);
13821382
}
13831383

1384-
class ExpiryLimitTest : public KVBucketTest {
1384+
class RelativeExpiryLimitTest : public KVBucketTest {
13851385
public:
13861386
void SetUp() override {
1387-
config_string += "max_ttl=86400";
1387+
config_string += "max_ttl=2592000";
13881388
KVBucketTest::SetUp();
13891389
// Have all the objects, activate vBucket zero so we can store data.
13901390
store->setVBucketState(vbid, vbucket_state_active, false);
13911391
}
13921392
};
13931393

13941394
// Test that item allocate with a limit stops 0 expiry
1395-
TEST_F(ExpiryLimitTest, itemAllocate) {
1395+
TEST_F(RelativeExpiryLimitTest, itemAllocate) {
13961396
item* itm;
13971397
EXPECT_EQ(ENGINE_SUCCESS,
13981398
engine->itemAllocate(&itm,
@@ -1412,7 +1412,7 @@ TEST_F(ExpiryLimitTest, itemAllocate) {
14121412
}
14131413

14141414
// Test that GAT with a limit stops 0 expiry
1415-
TEST_F(ExpiryLimitTest, gat) {
1415+
TEST_F(RelativeExpiryLimitTest, gat) {
14161416
// This will actually skip the initial expiry limiting code as this function
14171417
// doesn't use itemAllocate
14181418
Item item = store_item(
@@ -1429,6 +1429,42 @@ TEST_F(ExpiryLimitTest, gat) {
14291429
EXPECT_NE(0, info.exptime);
14301430
}
14311431

1432+
class AbsoluteExpiryLimitTest : public KVBucketTest {
1433+
public:
1434+
void SetUp() override {
1435+
config_string += "max_ttl=2592001";
1436+
KVBucketTest::SetUp();
1437+
// Have all the objects, activate vBucket zero so we can store data.
1438+
store->setVBucketState(vbid, vbucket_state_active, false);
1439+
}
1440+
};
1441+
1442+
TEST_F(AbsoluteExpiryLimitTest, MB_37643) {
1443+
// Go forwards by 30 days + 1000 seconds, we want uptime > max_ttl
1444+
const int uptime = (60 * 60 * 24 * 30) + 1000;
1445+
TimeTraveller biff(uptime);
1446+
item* itm;
1447+
EXPECT_EQ(ENGINE_SUCCESS,
1448+
engine->itemAllocate(&itm,
1449+
{"key", DocNamespace::DefaultCollection},
1450+
5,
1451+
0,
1452+
0,
1453+
0 /*expiry is 0*/,
1454+
0,
1455+
vbid));
1456+
1457+
Item* i = reinterpret_cast<Item*>(itm);
1458+
auto info = engine->getItemInfo(*i);
1459+
1460+
// We expect that the expiry time is in the future. The future here being
1461+
// current_time + our time shift.
1462+
1463+
// We expect that the expiry is at least now+uptime+max_ttl
1464+
EXPECT_GT(info.exptime, ep_abs_time(ep_current_time()));
1465+
engine->itemRelease(itm);
1466+
}
1467+
14321468
// Test cases which run for EP (Full and Value eviction) and Ephemeral
14331469
INSTANTIATE_TEST_CASE_P(EphemeralOrPersistent,
14341470
KVBucketParamTest,

programs/engine_testapp/mock_server.cc

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -261,9 +261,6 @@ static rel_time_t mock_realtime(rel_time_t exptime, cb::ExpiryLimit limit) {
261261
rv = (rel_time_t)(exptime + mock_get_current_time());
262262
}
263263

264-
if (limit && rv > limit.get().count()) {
265-
rv = gsl::narrow<rel_time_t>(limit.get().count());
266-
}
267264
return rv;
268265
}
269266

0 commit comments

Comments
 (0)