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

mon: fix mon_keyvaluedb application #15059

Merged
merged 1 commit into from May 17, 2017

Conversation

Projects
None yet
3 participants
@liewegas
Copy link
Member

liewegas commented May 12, 2017

In 42a6b0e I mistakenly thought that
create_or_open was the path taken for normal open (not during mkfs) and
made it assume a missing kv_type meant leveldb. In reality, this is the
only path where mon_keyvaluedb is ever used (during mkfs), and
create_or_open is only called during mkfs. The bug I was (probably?)
trying to fix was that regular open() did not write out a kv_type file
(with the assumption of leveldb) if it was missing. As a result, the
previous fix was forcing all mons to be leveldb.

Fix this by reverting the create_or_open hunk (so that we express
mon_keyvaluedb on mkfs), and fixing the normal open path to write kv_type
if it is missing. This effectively switches the mon back to rocksdb by
default (and allows teuthology to test both rocksdb and leveldb by setting
the option).

Signed-off-by: Sage Weil sage@redhat.com

@liewegas liewegas requested a review from tchaikov May 12, 2017

@tchaikov
Copy link
Contributor

tchaikov left a comment

lgtm modulo the nit

@@ -629,9 +629,13 @@ class MonitorDBStore
int open(ostream &out) {
string kv_type;
int r = read_meta("kv_backend", &kv_type);
if (r < 0 || kv_type.length() == 0)
if (r < 0 || kv_type.length() == 0) {

This comment has been minimized.

Copy link
@tchaikov

tchaikov May 12, 2017

Contributor

nit, kv_type.empty()

@tchaikov tchaikov added the needs-qa label May 12, 2017

mon: fix mon_keyvaluedb application
In 42a6b0e I mistakenly thought that
create_or_open was the path taken for normal open (not during mkfs) and
made it assume a missing kv_type meant leveldb.  In reality, this is the
only path where mon_keyvaluedb is ever used (during mkfs), and
create_or_open is only called during mkfs.  The bug I was (probably?)
trying to fix was that regular open() did not write out a kv_type file
(with the assumption of leveldb) if it was missing.  As a result, the
previous fix was forcing all mons to be leveldb.

Fix this by reverting the create_or_open hunk (so that we express
mon_keyvaluedb on mkfs), and fixing the normal open path to write kv_type
if it is missing.  This effectively switches the mon back to rocksdb by
default (and allows teuthology to test both rocksdb and leveldb by setting
the option).

Signed-off-by: Sage Weil <sage@redhat.com>

@liewegas liewegas force-pushed the liewegas:wip-mon-kvtype branch from c43fd64 to 675f6d9 May 12, 2017

@liewegas

This comment has been minimized.

@liewegas liewegas merged commit 0260a52 into ceph:master May 17, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.