fix(string): reset the value of expired key for SETRANGE cmd #2686
Conversation
git-hulk
changed the title
|
@weim0000 Thanks for your fix, would you mind adding a test case to cover this? https://github.com/apache/kvrocks/blob/unstable/tests/gocase/unit/type/strings/strings_test.go |
Ok, no problem. I will add the test cases ASAP |
src/types/redis_string.cc
Outdated
| @@ -297,6 +297,7 @@ rocksdb::Status String::SetRange(engine::Context &ctx, const std::string &user_k | |||
| } | |||
|
|
|||
| Metadata metadata(kRedisString, false); | |||
| raw_value.clear(); | |||
There was a problem hiding this comment.
I think it'd be better to clear the raw_value inside getRawValue if the return status isn't ok. Because we call getRawValue many times and it's hard to keep having an eye on this. @torwig @PragmaTwice @mapleFU What do you think?
There was a problem hiding this comment.
@git-hulk Valid point. Since the value has expired, we should return "nothing" (empty) to the caller of getRawValue.
There was a problem hiding this comment.
Good to me. Actually refactor them to StatusOr can solve the problem, but I don't have much time to do that.
…eturn status isn't ok
|
|
@weim0000 Thanks for your fix. |
reset the value of expired key to prevent the following parsing and calculation.
`127.0.0.1:6003> set k v
OK
127.0.0.1:6003> expire k 1
(integer) 1
after 2 seconds
127.0.0.1:6003> get k
(nil)
127.0.0.1:6003> setrange k 1 aa
(integer) 10
127.0.0.1:6003> get k
(nil)
127.0.0.1:6003> exists k
(integer) 0
127.0.0.1:6003>`
1 set value with expire time.
2 wait until the key expires
3 use setrange to this key. The cmd be successful, but get k return nil
What did you expect to see?
127.0.0.1:6003> set k v
OK
127.0.0.1:6003> expire k 1
(integer) 1
127.0.0.1:6003> get k
(nil)
127.0.0.1:6003> setrange k 0 aa
(integer) 10
127.0.0.1:6003> get k
aa
127.0.0.1:6003> exists k
(integer) 1
127.0.0.1:6003>