Conversation
manishrjain
left a comment
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @anurags92 and @karlmcguire)
manishrjain
left a comment
There was a problem hiding this comment.
This is good to go.
Reviewable status:
complete! all files reviewed, all discussions resolved (waiting on @anurags92 and @karlmcguire)
|
@martinmr I have a test where I add a value to cache with TTL and then check for that value after TTL passed(say one second), that test is failing after this change(works fine with v0.0.3 ofc). I had to increase my wait time to make that test pass. I don't see any change that would do that in quick pass over code, but it would be a good idea to check SetWithTTL tests after this change. |
|
Alright, so did some testing with couple sha's around this change, and this change definitely has some kind of regression with TTL. |
|
hey @varun06 , can you share your test? Maybe send a PR to add it to ristretto? We can look at the failing test if this change has caused it. |
|
sure @jarifibrahim |
|
@jarifibrahim This is the test that's failing |
|
so it seems that there is a regression when we use SetWithTTL. |
Related to DGRAPH-1378
This change is