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: support for building without leveldb + mon mkfs bug fix #11800

Merged
merged 2 commits into from Nov 13, 2016

Conversation

bassam
Copy link
Contributor

@bassam bassam commented Nov 5, 2016

WITH_LEVELDB=OFF is now fully supported and we will not link leveldb anywhere.

Also fixed an issue that prevented mon mkfs from creating a rocksdb store.

@tchaikov tchaikov self-assigned this Nov 6, 2016
@liewegas liewegas changed the title Support for building without leveldb + mon mkfs bug fix mon: support for building without leveldb + mon mkfs bug fix Nov 6, 2016
@liewegas liewegas added this to the kraken milestone Nov 6, 2016
@@ -241,10 +236,14 @@ option(WITH_KVS "Key value store is here" ON)
option(WITH_RBD "Remote block storage is here" ON)

option(WITH_LEVELDB "LevelDB is here" ON)
if(${WITH_LEVELDB})
if(WITH_LEVELDB)
if(LEVELDB_PREFIX)
Copy link
Contributor

Choose a reason for hiding this comment

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

@bassamtabbara could you indent the block in if(WITH_LEVELDB)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure.

@@ -140,7 +140,8 @@ int check_mon_data_empty()
errno = 0;
while ((de = ::readdir(dir))) {
if (string(".") != de->d_name &&
string("..") != de->d_name) {
string("..") != de->d_name &&
string("kv_backend") != de->d_name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@bassamtabbara could you explain a little bit on what is the use case for this change? "kv_backend" is created when the monitor db is created, so its existence implies that the monitor db is already created?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

kv_backend is not being created correctly monitor db is created and looking and the PendingReleaseNotes I'm not sure if thats the intention.

With the described semantics I was unable to find a way to create monitor with rocksdb.
ceph-mon.mkfs calls store.create_and_open (here https://github.com/ceph/ceph/blob/master/src/ceph_mon.cc#L409) and that expects the kv_backend file to already exist (see https://github.com/ceph/ceph/blob/master/src/mon/MonitorDBStore.h#L642). If I create the kv_backend file then check_mon_data_emtpy (here https://github.com/ceph/ceph/blob/master/src/ceph_mon.cc#L129) fails.

Copy link
Contributor

Choose a reason for hiding this comment

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

makes sense!

@bassam bassam force-pushed the wip-make-leveldb-optional branch 2 times, most recently from d677b05 to f89484f Compare November 7, 2016 17:10
@bassam
Copy link
Contributor Author

bassam commented Nov 7, 2016

@tchaikov fixed the formatting issues and rebased. I verified this by removing libleveldb-dev on my ubuntu box and building WITH_LEVELDB=OFF. all compiles fine. I was able to create a whole cluster using rocksdb for MON and OSDs.

@tchaikov
Copy link
Contributor

tchaikov commented Nov 8, 2016

lgtm.

now that monitors support rocksdb, fix the build system
to enable compiling without leveldb. kv_backend would need
to be set correctly if you were to to use rocksdb for the
monitors.

Signed-off-by: Bassam Tabbara <bassam.tabbara@quantum.com>
check_mon_data_empty required a completely empty dir. this
made it impossible to mkfs a mondir that uses rocksdb given
the requirement that kv_backend file exist in the mon datadir.

Signed-off-by: Bassam Tabbara <bassam.tabbara@quantum.com>
@tchaikov tchaikov merged commit 18d27b1 into ceph:master Nov 13, 2016
bassam added a commit to bassam/ceph that referenced this pull request Nov 15, 2016
ceph#11800 uninternionally updated
the civetweb submodule.

Signed-off-by: Bassam Tabbara <bassam.tabbara@quantum.com>
@bassam bassam deleted the wip-make-leveldb-optional branch November 22, 2016 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants