Set move key's expiresAt for keys with TTL#1006
Conversation
The value log GC moves keys from one value log file to another. The current implementation of vlog GC would not copy the TTL from the original entry to the new move key. This commit fixes that. Without this change the move keys were not being removed as seen in issue #974. With this commit, the move keys are being removed.
There was a problem hiding this comment.
✅ A review job has been created and sent to the PullRequest network.
@jarifibrahim you can click here to see the review status or cancel the code review job.
There was a problem hiding this comment.
Went through the PR as well as the related issue, no red flags with the single line change. I did notice your AppVeyor tests are failing which may or may not be immediately related to the change in this line of code which you should look into before merging.
Thanks for sharing as much details on the PR comment though, including the graphs, really makes it easy to review.
Glad this was a relatively short and sweet fix! 🙂
Reviewed with ❤️ by PullRequest
manishrjain
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1.
Reviewable status:complete! all files reviewed, all discussions resolved
The value log GC moves keys from one value log file to another. The current implementation of vlog GC would not copy the TTL from the original entry to the new move key. This commit fixes that. Without this change, the move keys were not being removed as seen in issue #974. With this commit, the move keys are being removed. (cherry picked from commit d1c0fba)
The value log GC moves keys from one value log file to another. The
current implementation of vlog GC would not copy the TTL from the
original entry to the new move key. This commit fixes that.
Without this change, the move keys were not being removed as seen in
issue #974.
With this commit, the move keys are being removed.
This script (https://gist.github.com/kung-foo/66317e4b274ec456a92270e32d692ff7) was used to generate the results shown below.
Move keys being removed because of this Patch (internal keys == move keys)
Move keys not being removed in master
Fixes: #974
This change is