Skip to content
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 memtable related files into memtable directory #2087

Closed
wants to merge 1 commit into from

Conversation

yiwu-arbug
Copy link
Contributor

Summary:
Move memtable related files into memtable directory.

Test Plan:
run existing tests.

@facebook-github-bot
Copy link
Contributor

@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@ajkr ajkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we want to remove memtable->db dependencies in the future?

@yiwu-arbug
Copy link
Contributor Author

@ajkr Good point! Will work on it.

@yiwu-arbug
Copy link
Contributor Author

The list of memtable->db dependencies:

memtable.cc:#include "db/dbformat.h"
memtable.cc:#include "db/merge_context.h"
memtable.cc:#include "db/merge_helper.h"
memtable.cc:#include "db/pinned_iterators_manager.h"
memtable.h:#include "db/dbformat.h"
memtable.h:#include "db/range_del_aggregator.h"
memtable.h:#include "db/version_edit.h"
memtablerep_bench.cc:#include "db/dbformat.h"

memtable.h contain too many dependencies from db. Shall we move it back to db directory?
And memtablerep_bench.cc is depending on InternalKeyComparator from db/dbformat.h. Do you think moving InternalKeyComparator to public header a good idea or not? Need suggestions.

@siying
Copy link
Contributor

siying commented Apr 6, 2017

@yiwu-arbug yes, maybe move it back. Just leave the skiplist rep.

@yiwu-arbug
Copy link
Contributor Author

@siying cool.

Summary:
Move memtable related files into memtable directory.

Test Plan:
run existing tests.
@facebook-github-bot
Copy link
Contributor

@yiwu-arbug updated the pull request - view changes - changes since last import

@yiwu-arbug
Copy link
Contributor Author

Will land after windows build pass.

@facebook-github-bot
Copy link
Contributor

@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

mikhail-antonov pushed a commit to mikhail-antonov/rocksdb that referenced this pull request Apr 12, 2017
Summary:
Move memtable related files into memtable directory.
Closes facebook#2087

Differential Revision: D4829242

Pulled By: yiwu-arbug

fbshipit-source-id: ca70ab6
@yiwu-arbug yiwu-arbug deleted the memtable branch July 19, 2018 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants