Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Round expire time for seconds to improve precision for 32bit-common-field encoding #1352

Merged
merged 2 commits into from
Mar 24, 2023
Merged

Round expire time for seconds to improve precision for 32bit-common-field encoding #1352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1536a21
Round expire time for seconds to improve precision for 32bit-common-f…
PragmaTwice Mar 24, 2023
467056c
fix
PragmaTwice Mar 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/commands/cmd_key.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,13 @@ class CommandExists : public Commander {
class CommandExpire : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
seconds_ = GET_OR_RET(ParseInt<int64_t>(args[2], 10)) + Util::GetTimeStamp();
ttl_ = GET_OR_RET(ParseInt<int64_t>(args[2], 10));
return Status::OK();
}

Status Execute(Server *svr, Connection *conn, std::string *output) override {
Redis::Database redis(svr->storage_, conn->GetNamespace());
auto s = redis.Expire(args_[1], seconds_ * 1000);
auto s = redis.Expire(args_[1], ttl_ * 1000 + Util::GetTimeStampMS());
if (s.ok()) {
*output = Redis::Integer(1);
} else {
Expand All @@ -131,7 +131,7 @@ class CommandExpire : public Commander {
}

private:
uint64_t seconds_ = 0;
uint64_t ttl_ = 0;
};

class CommandPExpire : public Commander {
Expand Down
4 changes: 2 additions & 2 deletions src/storage/redis_db.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,13 +324,13 @@ rocksdb::Status Database::RandomKey(const std::string &cursor, std::string *key)

std::string end_cursor;
std::vector<std::string> keys;
auto s = Scan(cursor, 60, "", &keys, &end_cursor);
auto s = Scan(cursor, RANDOM_KEY_SCAN_LIMIT, "", &keys, &end_cursor);
if (!s.ok()) {
return s;
}
if (keys.empty() && !cursor.empty()) {
// if reach the end, restart from beginning
s = Scan("", 60, "", &keys, &end_cursor);
s = Scan("", RANDOM_KEY_SCAN_LIMIT, "", &keys, &end_cursor);
if (!s.ok()) {
return s;
}
Expand Down
2 changes: 2 additions & 0 deletions src/storage/redis_db.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
namespace Redis {
class Database {
public:
static constexpr uint64_t RANDOM_KEY_SCAN_LIMIT = 60;

explicit Database(Engine::Storage *storage, std::string ns = "");
rocksdb::Status GetMetadata(RedisType type, const Slice &ns_key, Metadata *metadata);
rocksdb::Status GetRawMetadata(const Slice &ns_key, std::string *bytes);
Expand Down
3 changes: 2 additions & 1 deletion src/storage/redis_metadata.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,8 @@ uint64_t Metadata::ExpireMsToS(uint64_t ms) {
return 1;
}

return ms / 1000;
// We use rounding to get closer to the original value
return (ms + 499) / 1000;
}

bool Metadata::Is64BitEncoded() const { return flags & METADATA_64BIT_ENCODING_MASK; }
Expand Down
8 changes: 4 additions & 4 deletions tests/cppunit/types/string_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ TEST_F(RedisStringTest, MSetXX) {
EXPECT_EQ(ret, 1);
int64_t ttl = 0;
string->TTL(key_, &ttl);
EXPECT_TRUE(ttl >= 2000 && ttl <= 3000);
EXPECT_TRUE(ttl >= 2000 && ttl <= 4000);
string->Del(key_);
}

Expand Down Expand Up @@ -214,15 +214,15 @@ TEST_F(RedisStringTest, MSetNXWithTTL) {
string->SetNX(key_, "test-value", 3000, &ret);
int64_t ttl = 0;
string->TTL(key_, &ttl);
EXPECT_TRUE(ttl >= 2000 && ttl <= 3000);
EXPECT_TRUE(ttl >= 2000 && ttl <= 4000);
string->Del(key_);
}

TEST_F(RedisStringTest, SetEX) {
string->SetEX(key_, "test-value", 3000);
int64_t ttl = 0;
string->TTL(key_, &ttl);
EXPECT_TRUE(ttl >= 2000 && ttl <= 3000);
EXPECT_TRUE(ttl >= 2000 && ttl <= 4000);
string->Del(key_);
}

Expand Down Expand Up @@ -277,7 +277,7 @@ TEST_F(RedisStringTest, CAS) {

int64_t ttl = 0;
string->TTL(key, &ttl);
EXPECT_TRUE(ttl >= 9000 && ttl <= 10000);
EXPECT_TRUE(ttl >= 9000 && ttl <= 11000);

string->Del(key);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/gocase/unit/expire/expire_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func TestExpire(t *testing.T) {
t.Run("PTTL returns time to live in milliseconds", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "x").Err())
require.NoError(t, rdb.SetEx(ctx, "x", "somevalue", 2*time.Second).Err())
util.BetweenValues(t, rdb.PTTL(ctx, "x").Val(), 50*time.Millisecond, 2000*time.Millisecond)
util.BetweenValues(t, rdb.PTTL(ctx, "x").Val(), 500*time.Millisecond, 2500*time.Millisecond)
})

t.Run("TTL / PTTL return -1 if key has no expire", func(t *testing.T) {
Expand Down