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
clean up KeyValueDB instantiation #2163
Conversation
Allow callers to set config values even when there is no observer. Signed-off-by: Sage Weil <sage@inktank.com>
Let us create an implemenetation by name. Include a test_init() method that will instantiate an instance and verify it could start up. Signed-off-by: Sage Weil <sage@inktank.com>
Signed-off-by: Sage Weil <sage@inktank.com>
Signed-off-by: Sage Weil <sage@inktank.com>
… superblock The only time we set this is at mkfs time. Thereafter, we stick with the same backend. Signed-off-by: Sage Weil <sage@redhat.com>
This option should be keyvaluestore_*, not osd_*. Clean up the backend instantiation. Signed-off-by: xinxin shu <xinxin.shu@intel.com> Signed-off-by: Sage Weil <sage@redhat.com>
KeyValueDB *KeyValueDB::create(CephContext *cct, const string& type, | ||
const string& dir) | ||
{ | ||
if (type == "leveldb") { |
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.
Shouldn't the type be encapsulated by the KeyValueDB create member function? I think the caller would not care as long as they received an instance of something with the same interface as KeyValueDB. But this is just a suggetion.
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.
You mean they shouldn't need to pass in the const string& type? That's needed because various config options control which implementation is used in different contexts (mon, osd filestore -- based on what is recorded on disk or based on the config option at mkfs time, osd keyvaluestore, etc.). Or do you mean something else?
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.
Ah, I understand now. The caller knows the type of KVDB it needs, and passes that information to KeyValueDB().
It's good to me! |
clean up KeyValueDB instantiation Reviewed-by: Haomai Wang <haomaiwang@gmail.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
This cleans up some of the kinetic stuff and lays things out for rocksdb.