Skip to content

Commit

Permalink
#260 Accept lowercase options expire_in expire_at. Improve the error …
Browse files Browse the repository at this point in the history 
…messages when the absolute timestamp is incorrect

Summary:
Currently getting these errors:

127.0.0.1:6379> TSADD cpu_usage 70 "80" expire_at 1
(error) ERR TSADD: expire_at is not a valid number

127.0.0.1:6379> TSADD cpu_usage 70 "80" expire_in 1
(error) ERR TSADD: expire_in is not a valid number

127.0.0.1:6379> TSADD cpu_usage 70 "80" EXPIRE_AT 1513642307
(error) ERR TSADD: TTL: -11469038 needs be in the range [1, 9223372036]

Test Plan:
New unit tests

Also, for the incorrect error message, it has changed to:

127.0.0.1:6379> TSADD cpu_usage 70 "80" EXPIRE_AT 1513642307
(error) ERR TSADD: TTL: 1513642307 needs be in the range [1525208329, 10748580364]

Reviewers: pritam.damania, kannan, rahuldesirazu, amitanand

Reviewed By: amitanand

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D4734
  • Loading branch information
@hectorgcr
hectorgcr committed May 3, 2018
1 parent 91123ee commit 01ef49b
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 17 deletions.
49 changes: 33 additions & 16 deletions src/yb/yql/redis/redisserver/redis_parser.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,23 +257,40 @@ CHECKED_STATUS ParseHMSetLikeCommands(YBRedisWriteOp *op, const RedisClientComma
std::unordered_map<string, string> kv_map;
for (int i = start_idx; i < args.size(); i += 2) {
// EXPIRE_AT/EXPIRE_IN only supported for redis timeseries currently.
if ((args[i] == kExpireAt || args[i] == kExpireIn) && type == REDIS_TYPE_TIMESERIES) {
if (i + 2 != args.size()) {
return STATUS_SUBSTITUTE(InvalidCommand, "$0 should be at the end of the command",
string(args[i].cdata(), args[i].size()));
if (type == REDIS_TYPE_TIMESERIES) {
string upper_arg;
ToUpperCase(args[i].ToBuffer(), &upper_arg);
if (upper_arg == kExpireAt || upper_arg == kExpireIn) {
if (i + 2 != args.size()) {
return STATUS_SUBSTITUTE(InvalidCommand, "$0 should be at the end of the command",
string(args[i].cdata(), args[i].size()));
}
auto temp = util::CheckedStoll(args[i + 1]);
RETURN_NOT_OK(temp);
int64_t ttl = 0;
if (upper_arg == kExpireIn) {
ttl = *temp;
if (ttl > kRedisMaxTtlSeconds || ttl < kRedisMinTtlSeconds) {
return STATUS_SUBSTITUTE(InvalidCommand, "TTL: $0 needs be in the range [$1, $2]", ttl,
kRedisMinTtlSeconds, kRedisMaxTtlSeconds);
}
} else {
auto current_time = GetCurrentTimeMicros() / MonoTime::kMicrosecondsPerSecond;
ttl = *temp - current_time;
if (ttl > kRedisMaxTtlSeconds || ttl < kRedisMinTtlSeconds) {
return STATUS_SUBSTITUTE(InvalidCommand, "EXPIRE_AT: $0 needs be in the range [$1, $2]",
*temp,
kRedisMinTtlSeconds + current_time,
kRedisMaxTtlSeconds + current_time);
}
}

// Need to pass ttl in milliseconds, user supplied values are in seconds.
op->mutable_request()->mutable_set_request()->set_ttl(
ttl * MonoTime::kMillisecondsPerSecond);
} else {
kv_map[args[i].ToBuffer()] = args[i + 1].ToBuffer();
}
auto temp = util::CheckedStoll(args[i + 1]);
RETURN_NOT_OK(temp);
auto ttl = args[i] == kExpireIn
? *temp
: *temp - GetCurrentTimeMicros() / MonoTime::kMicrosecondsPerSecond;

if (ttl > kRedisMaxTtlSeconds || ttl < kRedisMinTtlSeconds) {
return STATUS_SUBSTITUTE(InvalidCommand, "TTL: $0 needs be in the range [$1, $2]", ttl,
kRedisMinTtlSeconds, kRedisMaxTtlSeconds);
}
// Need to pass ttl in milliseconds, user supplied values are in seconds.
op->mutable_request()->mutable_set_request()->set_ttl(ttl * MonoTime::kMillisecondsPerSecond);
} else if (type == REDIS_TYPE_SORTEDSET) {
// For sorted sets, we store the mapping from values to scores, since values are distinct
// but scores aren't.
Expand Down
16 changes: 15 additions & 1 deletion src/yb/yql/redis/redisserver/redisserver-test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1701,7 +1701,7 @@ TEST_F(TestRedisService, TestZRange) {
}

TEST_F(TestRedisService, TestTimeSeriesTTL) {
int64_t ttl_sec = 5;
int64_t ttl_sec = 10;
TestTSTtl("EXPIRE_IN", ttl_sec, ttl_sec, "test_expire_in");
int64_t curr_time_sec = GetCurrentTimeMicros() / MonoTime::kMicrosecondsPerSecond;
TestTSTtl("EXPIRE_AT", ttl_sec, curr_time_sec + ttl_sec, "test_expire_at");
Expand All @@ -1712,9 +1712,23 @@ TEST_F(TestRedisService, TestTimeSeriesTTL) {
std::to_string(kRedisMaxTtlSeconds + 1)});

DoRedisTestExpectError(__LINE__, {"TSADD", "test_expire_at", "10", "v1", "EXPIRE_AT",
std::to_string(curr_time_sec - 10)});
DoRedisTestExpectError(__LINE__, {"TSADD", "test_expire_at", "10", "v1", "expire_at",
std::to_string(curr_time_sec - 10)});

DoRedisTestExpectError(__LINE__, {"TSADD", "test_expire_at", "10", "v1", "EXPIRE_AT",
std::to_string(curr_time_sec + kRedisMinTtlSeconds - 1)});
DoRedisTestExpectError(__LINE__, {"TSADD", "test_expire_at", "10", "v1", "expire_at",
std::to_string(curr_time_sec + kRedisMinTtlSeconds - 1)});
DoRedisTestExpectError(__LINE__, {"TSADD", "test_expire_at", "10", "v1", "exPiRe_aT",
std::to_string(curr_time_sec + kRedisMinTtlSeconds - 1)});

DoRedisTestExpectError(__LINE__, {"TSADD", "test_expire_at", "10", "v1", "EXPIRE_IN",
std::to_string(curr_time_sec + kRedisMaxTtlSeconds + 1)});
DoRedisTestExpectError(__LINE__, {"TSADD", "test_expire_at", "10", "v1", "expire_in",
std::to_string(curr_time_sec + kRedisMaxTtlSeconds + 1)});
DoRedisTestExpectError(__LINE__, {"TSADD", "test_expire_at", "10", "v1", "eXpIrE_In",
std::to_string(curr_time_sec + kRedisMaxTtlSeconds + 1)});
}

TEST_F(TestRedisService, TestTsCard) {
Expand Down

0 comments on commit 01ef49b

Please sign in to comment.