feat: add values and keys #11
Conversation
mehrantsi
left a comment
There was a problem hiding this comment.
Thanks for the PR! A few issues to address:
- When caching is enabled, keys()/values() returns only cached keys, not all keys. The cache is a hot-data subset, not the full dataset. We also should consider scanning hash_table (or tree for sorted order).
- The stats hit/miss tracking also doesn't quite fit these bulk operations. You might want to simplify
P.S: When changed to iterate the hash_table or tree, you also need to consider the case that feox is running in persisted mode and value is None. In that case, you'll need to load value from disk. Given the size, I think Iter() method would be more efficient.
Would you like to continue on this or should I push fixes? :)
| let start = std::time::Instant::now(); | ||
|
|
||
| if self.enable_caching { | ||
| if let Some(ref cache) = self.cache { |
There was a problem hiding this comment.
I see that we're iterating keys from cache. but this is only a subset of keys, not all of them. is that intentional?
| self.keys_latency_ns | ||
| .fetch_add(latency_ns, Ordering::Relaxed); | ||
|
|
||
| if hit { |
There was a problem hiding this comment.
since you're reading keys/values from cache, cache hit/miss counter updates are misleading. also this isn't really a "get" operation, rather a bulk get operation; so I'm not sure about increasing the get counter either.
If it takes 5 mins for you, you can continue 😄 ; if not, then I can fix them. I can continue on the weekend next. |
|
@dancixx pretty sure it'll be more than 5 minutes 😄 alright, then feel free to continue whenever you have time. Thanks! |
No description provided.