-
Notifications
You must be signed in to change notification settings - Fork 6.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move FIFOCompactionPicker to a separate file #4724
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sagar0 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks to @sagar0 for re-organizing the code.
I left a minor comment.
@sagar0 has updated the pull request. Re-import the pull request |
@sagar0 has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sagar0 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@riversand963 can you take another look? I updated based on your suggestion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM again!
Summary: **Summary:** Simplified the code layout by moving FIFOCompactionPicker to a separate file. **Why?:** While trying to add ttl functionality to universal compaction, I found that `FIFOCompactionPicker` class and its impl methods to be interspersed between `LevelCompactionPicker` methods which kind-of made the code a little hard to traverse. So I moved `FIFOCompactionPicker` to a separate compaction_picker_fifo.h/cc file, similar to `UniversalCompactionPicker`. Pull Request resolved: facebook#4724 Differential Revision: D13227914 Pulled By: sagar0 fbshipit-source-id: 89471766ea67fa4d87664a41c057dd7df4b3d4e3
Summary:
Simplified the code layout by moving FIFOCompactionPicker to a separate file.
Why?:
While trying to add ttl functionality to universal compaction, I found that
FIFOCompactionPicker
class and its impl methods to be interspersed betweenLevelCompactionPicker
methods which kind-of made the code a little hard to traverse. So I movedFIFOCompactionPicker
to a separate compaction_picker_fifo.h/cc file, similar toUniversalCompactionPicker
.Test Plan:
make check