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

mv-iterator-prev branch #69

Merged
merged 3 commits into from
Sep 6, 2013
Merged

mv-iterator-prev branch #69

merged 3 commits into from
Sep 6, 2013

Conversation

matthewvon
Copy link
Contributor

This branch is to address issue: #52

The async NIF release knowingly disable the iterator "prev" support. Implicitly it also disabled all "next" / "prev" combinations. The assumption then was the Riak only used "fold" ... so don't sweat the details. However, other folks use eleveldb outside of Riak and need the support ... this restores the feature.

@ghost ghost assigned matthewvon Aug 13, 2013
@uwiger
Copy link

uwiger commented Aug 27, 2013

Any ETC on this pull request? I'm doing integration this week, and it would be nice to have it. :)

@matthewvon
Copy link
Contributor Author

uwiger, I was hoping to hear back on the results in your environment ... and we have gotten into a bottleneck on code reviews here. I can likely get this out Thursday.

@uwiger
Copy link

uwiger commented Aug 27, 2013

I have been testing it with the mnesia test suite (automatically transforming all occurrences of disc_only_copies to use a leveldb backend instead. While this doesn't work 100%, the mv-iterator-prev branch performs at least as well as master (slightly better since prev() actually works).

The mnesia test suite shakes things up quite a lot, but operates on fairly small data sets. I have not yet run tests on this branch with really large tables.

@matthewvon
Copy link
Contributor Author

"While this doesn't work 100%" ... what is not working 100%: eleveldb, amnesia test suite, something else?

@uwiger
Copy link

uwiger commented Aug 28, 2013

Sorry for being unclear. The thing that doesn't work 100% is the "transforming all occurrences of disc_only_copies to use a leveldb backend instead" in the mnesia test suite.

@bookshelfdave
Copy link
Contributor

@matthewvon can you make Travis happy?

@matthewvon
Copy link
Contributor Author

Travis will not be consistently happy with this branch until 1) eleveldb's mv-clean-overlaps is pushed to master and 2) leveldb's mv-valgrind-cleanup is pushed to master. Travis is failing on the RSS test which Jon describes as unreliable. However, I reviewed it sufficiently to find a memory leak that lead to the other two branches. So, we could pull both branches to here, or finish the code review on the other two branches. I recommend the latter ... or ignoring Travis error on RSS.

@@ -540,8 +541,7 @@ ERL_NIF_TERM send_reply(ErlNifEnv *env, ERL_NIF_TERM ref, ERL_NIF_TERM reply)
itr_ptr.assign(ItrObject::RetrieveItrObject(env, itr_handle_ref));

// prefetch logic broke PREV and not fixing for Riak
Copy link
Contributor

Choose a reason for hiding this comment

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

is this comment still accurate?

@bookshelfdave
Copy link
Contributor

+1

Test output:

[0]R15B01p:[mv-iterator-prev]prime:~/basho/eleveldb$ ./rebar eunit
==> eleveldb (eunit)
======================== EUnit ========================
module 'basho_bench_driver_eldb'
cacheleak: cacheleak_test_ (module 'cacheleak')...RSS1: 237176
RSS1: 285028
RSS1: 272244
RSS1: 290236
RSS1: 292764
RSS1: 291308
RSS1: 298684
RSS1: 289360
RSS1: 299124
RSS1: 305092
[28.955 s] ok
module 'cleanup'
  cleanup: assumption_test...assumption_test: top
assumption_test: bottom
[0.526 s] ok
  cleanup: open_close_test...[1.007 s] ok
  cleanup: open_exit_test...[1.507 s] ok
  cleanup: iterator_test...[2.697 s] ok
  cleanup: iterator_db_close_test...[2.033 s] ok
  cleanup: iterator_exit_test...[1.511 s] ok
  [done in 9.298 s]
module 'eleveldb'
  eleveldb: open_test...ok
  eleveldb: fold_test...ok
  eleveldb: fold_keys_test...ok
  eleveldb: fold_from_key_test...ok
  eleveldb: destroy_test...ok
  eleveldb: compression_test...ok
  eleveldb: close_test...ok
  eleveldb: close_fold_test...ok
  [done in 0.025 s]
module 'eleveldb_bump'
iterators: prev_test (module 'iterators')...[0.012 s] ok
=======================================================
  All 16 tests passed.

matthewvon pushed a commit that referenced this pull request Sep 6, 2013
@matthewvon matthewvon merged commit 66aae23 into master Sep 6, 2013
@uwiger
Copy link

uwiger commented Sep 6, 2013

Thanks.

@seancribbs seancribbs deleted the mv-iterator-prev branch April 1, 2015 22:45
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

3 participants