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

kv: In memory keyvalue db implementation #9933

Merged
merged 1 commit into from Jul 14, 2016
Merged

Conversation

chhabaramesh
Copy link
Contributor

In memory key value db implementation with google btree map.
First version with minimal functional implementation.

Signed-off-by: Ramesh Chander Ramesh.Chander@sandisk.com

@chhabaramesh
Copy link
Contributor Author

@liewegas please review and merge.

@markhpc
Copy link
Member

markhpc commented Jun 27, 2016

sweeeeet! Thanks Ramesh! Very excited to play with this.

@chhabaramesh
Copy link
Contributor Author

@markhpc you are welcome. But this is just start, might have to tune it for concurrency.

@chhabaramesh chhabaramesh force-pushed the master branch 3 times, most recently from 2b125a9 to 23c870c Compare June 27, 2016 17:15
@chhabaramesh
Copy link
Contributor Author

@liewegas All tests in ceph_test_objectstore and ceph_test_keyvaluedb passes now. Only problem was the libsnappy issue.

@markhpc
Copy link
Member

markhpc commented Jun 29, 2016

@chhabaramesh got a chance to test this today. A couple of observations:

  1. We need to modify the CMakeLists.txt file otherwise the linker get's angry when trying to build this with cmake. I've done this here: markhpc@5318bb6

  2. Pretty quickly I'm hitting segfaults on the OSDs when using this with bluestore. I've included a snippet from gdb:

MemDBStore_Segfault.txt

@chhabaramesh
Copy link
Contributor Author

@markhpc , Thanks, will look in to it today.

@chhabaramesh
Copy link
Contributor Author

@markhpc , I fixed the make file issue as suggested.

When do you hit segfault? Can you please tell exact test you are doing?

@markhpc
Copy link
Member

markhpc commented Jun 29, 2016

@chhabaramesh segfault happened about 0.3s after OSD initialization, but may only be on the first couple of OSDs (at least it is in this test). Looks like the segfaults probably happened during initial default pool creation.

@liewegas
Copy link
Member

liewegas commented Jun 29, 2016 via email

@markhpc
Copy link
Member

markhpc commented Jun 29, 2016

Did a quick test with mon_keyvaluedb set to memdb which results in:

2016-06-29 11:42:38.494078 7f172b662500 -1 unable to read magic from mon data

No segfaults, mon just doesn't appear to like it. Even with debug 20 there's not much in the mon log to look at.

@chhabaramesh
Copy link
Contributor Author

chhabaramesh commented Jun 29, 2016

@markhpc @liewegas , So far I did

  1. ceph_test_objectstore + ceph_test_keyvaluedb
  2. Vstart and then single rbd image and fio on top of it

for me it works ok without segfalut.

please let me do more tests , will update you ..

@chhabaramesh chhabaramesh force-pushed the master branch 4 times, most recently from 094a7dd to 9ec68bb Compare July 1, 2016 05:56

#define CEPH_MS_CONTAINER_NAME "ceph_kv_store"

class MSStore : public KeyValueDB
Copy link
Member

Choose a reason for hiding this comment

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

Can we call it MemDB{.h,.cc}?

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, will change it.


std::string MSStore::_get_data_fn()
{
string fn = m_db_path+"//"+"MemDB.db";
Copy link
Member

Choose a reason for hiding this comment

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

single /, spaces between operators and operands

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do that.

@liewegas
Copy link
Member

liewegas commented Jul 1, 2016

I pushed several patches to https://github.com/liewegas/ceph/commits/wip-mondb.. enough to get the monitor to get most of the way into its startup sequence with 'mon keyvaluedb = memdb'. But it quickly crashes in the iterator code.

@liewegas
Copy link
Member

liewegas commented Jul 2, 2016

Ah, I disabled the snapshot iterator usage in mon and now it works!


int MSStore::_rmkey(ms_op_t &op)
{
std::lock_guard<std::mutex> l(m_lock);
Copy link
Contributor

Choose a reason for hiding this comment

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

could shrink critical section (and elsewhere)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I will take utility code out of lock and break code wherever possible. Also, I have in mind overall concurrency improvement for this code using multiple btrees.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only thing I can take out of lock is combine strings. Rest needs to be under the lock. For example getting old value and then erasing it needs to be atomic, otherwise erase might fault.

@chhabaramesh chhabaramesh force-pushed the master branch 4 times, most recently from aa58360 to a0e41b4 Compare July 11, 2016 08:33
@liewegas
Copy link
Member

We need to test and merge #10102 first, then rebase this on top.

@liewegas
Copy link
Member

Otherwise this looks good to me!

@tchaikov
Copy link
Contributor

#10102 is merged.

@chhabaramesh
Copy link
Contributor Author

chhabaramesh commented Jul 13, 2016

@markhpc @tchaikov @liewegas rebased and tested. Passes ceph_test_objectstore + ceph_test_keyvaluedb + normal single node osd start and io.
Also did vstart with mondb and bluestore kv = memdb. It comes up and I can do basic read write test.

@chhabaramesh chhabaramesh force-pushed the master branch 2 times, most recently from 598fe9f to 0eac96b Compare July 13, 2016 08:41
Signed-off-by: Ramesh Chander <Ramesh.Chander@sandisk.com>
@markhpc
Copy link
Member

markhpc commented Jul 13, 2016

Sounds to me like we should merge it!

@chhabaramesh
Copy link
Contributor Author

@markhpc @tchaikov any update on this?

@tchaikov tchaikov merged commit c5e5ef7 into ceph:master Jul 14, 2016
@mattbenjamin
Copy link
Contributor

Yes, I will take utility code out of lock and break code wherever possible. Also, I have in mind overall > concurrency improvement for this code using multiple btrees.

The TreeX class in cohort_lru.h does something along those lines, but is based on the boost::intrusve tree interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants