added support for RANDOMKEY #303
Conversation
|
Please rebase on latest master and resolve the conflicts. Will review once done. |
core/store.go
Outdated
| @@ -328,3 +329,18 @@ func delByPtr(ptr unsafe.Pointer) bool { | |||
| } | |||
| return false | |||
| } | |||
|
|
|||
| func RandomKey() string { | |||
| if len(keypool) == 0 { | |||
There was a problem hiding this comment.
You need to acquire a lock when dealing with the keypool
core/store.go
Outdated
| } | ||
| index := rand.Intn(len(keypool)) | ||
| currentIndex := 0 | ||
| for key := range keypool { |
There was a problem hiding this comment.
Wouldn't this operation have O(N) complexity? If so there may be a chance of higher latency when the key size is large?
Wouldn't limiting the numbers of iterations to >=1024 be enough?
The time-complexity is REDIS is mentioned as O(1). Do we know how is it implemented in Redis?
There was a problem hiding this comment.
@JyotinderSingh & @soumya-codes I might need some help over here. To implement this RANDOMKEY command. I coundn't able to come up with any solution for O(1) complexity
There was a problem hiding this comment.
How about taking a very simplistic approach and limiting the number of iterations to within 512/1024.
idxRange := len(keypool) / 2
if idxRange > 1024 {
idxRange = 1024
}
rand.Seed(time.Now().UnixNano())
randomIndex := rand.Intn(idxRange)
@JyotinderSingh wdyt?
There was a problem hiding this comment.
This approach sounds good to me. But it will introduce a very heavy sampling bias towards the start of the keyset.
But I think it is an acceptable trade off at this stage, we could revisit this if we decide to modify our store data structures in the future.
core/store.go
Outdated
| @@ -328,3 +329,18 @@ func delByPtr(ptr unsafe.Pointer) bool { | |||
| } | |||
| return false | |||
| } | |||
|
|
|||
| func RandomKey() string { | |||
There was a problem hiding this comment.
Do we know if accessing a key randomly effects the expiry of the key?
There was a problem hiding this comment.
@soumya-codes yes RANDOMKEY should not return expired keys (with maxtries of 100, if exceeds max try it may return the expired key)
#254 added support for RANDOMKEY Command