-
Notifications
You must be signed in to change notification settings - Fork 233
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
[WIP] single instance leveldb for all qbufs #1601
Conversation
Thanks @hmmr! Settings---
minimum_reviewers: 2
merge: true
build_steps:
- make clean
- make deps
- make compile
- make test
- make xref
- make dialyzer
org_mode: true
timeout: 1800 |
There seems to be an issue with build step **make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
✅ MAKE_COMPILE
✅ MAKE_TEST
✅ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
c7c1a02
to
24c5818
Compare
There seems to be an issue with build step **make_compile,make_test,make_xref,make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
⛔ MAKE_COMPILE
⛔ MAKE_TEST
⛔ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
There seems to be an issue with build step **make_compile,make_test,make_xref,make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
⛔ MAKE_COMPILE
⛔ MAKE_TEST
⛔ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
There seems to be an issue with build step **make_compile,make_test,make_xref,make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
⛔ MAKE_COMPILE
⛔ MAKE_TEST
⛔ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
79addc8
to
6b69789
Compare
There seems to be an issue with build step **merge,make_dialyzer** ! ☁️⛔ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
✅ MAKE_COMPILE
✅ MAKE_TEST
✅ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
Work mostly done; putting it on hold pending leveldb changes from @matthewvon. |
There seems to be an issue with build step **merge,make_test,make_dialyzer** ! ☁️⛔ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
✅ MAKE_COMPILE
⛔ MAKE_TEST
✅ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
There seems to be an issue with build step **merge,make_compile,make_test,make_xref,make_dialyzer** ! ☁️⛔ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
⛔ MAKE_COMPILE
⛔ MAKE_TEST
⛔ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
6b69789
to
59f4731
Compare
There seems to be an issue with build step **make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
✅ MAKE_COMPILE
✅ MAKE_TEST
✅ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
59f4731
to
7a86559
Compare
There seems to be an issue with build step **make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
✅ MAKE_COMPILE
✅ MAKE_TEST
✅ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
7a86559
to
784fdb6
Compare
There seems to be an issue with build step **make_test,make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
✅ MAKE_COMPILE
⛔ MAKE_TEST
✅ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
There seems to be an issue with build step **make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
✅ MAKE_COMPILE
✅ MAKE_TEST
✅ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
There seems to be an issue with build step **make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
✅ MAKE_COMPILE
✅ MAKE_TEST
✅ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
%% total records stored (when complete) | ||
total_records = 0 :: non_neg_integer(), | ||
|
||
%% %% iterator cache (need a working iterator support in eleveldb) | ||
%% TODO: iterator cache |
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.
Not sure what you think an "iterator cache" will be, but it could be a horrible thing ... leaving you in a position of never finding data. Iterators lock in a "snapshot" of the database. You likely want a fresh snapshot AFTER the query data finishes posting.
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.
The lifecycle of each qbuf table is such that after writing to it is completed, it becomes a read-only. table (effectively, the last snapshot by your definition?). This is when I intended to open some number of iterators, with the idea to have a map of {Pos, Iter}
, so that when I need to fulfill a query with OFFSET N, I could pick the iterator whose associated Pos
is closest to N
and use it instead of iterating from the beginning.
However, there is no pressing need to develop on this further as query buffers are currently not reusable.
There seems to be an issue with build step **make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
✅ MAKE_COMPILE
✅ MAKE_TEST
✅ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
* parens around bools in guards; * no initialization of new vars inside if statement; * prefer `List /= []` over `length(List) > 0`.
# Conflicts: # src/riak_kv_qry_compiler.erl # src/riak_kv_ts_error_msgs.hrl
There seems to be an issue with build step **make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
✅ MAKE_COMPILE
✅ MAKE_TEST
✅ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
There seems to be an issue with build step **make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
✅ MAKE_COMPILE
✅ MAKE_TEST
✅ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
# Conflicts: # src/riak_kv_qry_compiler.erl
There seems to be an issue with build step **make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
✅ MAKE_COMPILE
✅ MAKE_TEST
✅ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
# Conflicts: # src/riak_kv_qry_compiler.erl
…ldb_for_qbufs # Conflicts: # src/riak_kv_qry_buffers.erl # src/riak_kv_qry_buffers_ldb.erl
There seems to be an issue with build step **make_compile,make_test,make_xref,make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
⛔ MAKE_COMPILE
⛔ MAKE_TEST
⛔ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
FYI: I do have the first half of the leveldb/eleveldb code posted and ready. I cannot start the second half until all the reviewers finish with the bucket expiry branches. The code needed to request query id status is still being debated by the Erlangers. The request mechanism is common to both bucket expiry and query buffers. You will NOT directly use this option. But just letting you know I really have done a little of the work: https://github.com/basho/leveldb/wiki/mv-disable-recovery-log I will use this option during the generation of a special ExpiryModule for the query buffers. |
There seems to be an issue with build step **merge,make_compile,make_test,make_xref,make_dialyzer** ! ☁️⛔ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
⛔ MAKE_COMPILE
⛔ MAKE_TEST
⛔ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
There seems to be an issue with build step **merge,make_compile,make_test,make_xref,make_dialyzer** ! ☁️⛔ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
⛔ MAKE_COMPILE
⛔ MAKE_TEST
⛔ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
…r_qbufs # Conflicts: # src/riak_kv_qry_buffers.erl # src/riak_kv_qry_buffers_ldb.erl
There seems to be an issue with build step **make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
✅ MAKE_COMPILE
✅ MAKE_TEST
✅ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
There seems to be an issue with build step **make_test,make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
✅ MAKE_COMPILE
⛔ MAKE_TEST
✅ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
There seems to be an issue with build step **make_dialyzer** ! ☁️✅ MERGE
✅ MAKE_CLEAN
✅ MAKE_DEPS
✅ MAKE_COMPILE
✅ MAKE_TEST
✅ MAKE_XREF
⛔ MAKE_DIALYZER
⬜ 0 of 2 Code reviews from organization basho |
https://bashoeng.atlassian.net/browse/RTS-1690
In 1.5.0, each query buffer is kept in a separate leveldb table. This involves many a file and dir creation/deletion ops, with the only benefit that dropping operation is as easy as
eleveldb:close() andalso rmdir()
.With automatic bucket expiry becoming available in leveldb, we can keep a single leveldb instance always open (never having to call
eleveldb:open
ourself) and, now storing query results in{Bucket, Key}
, offload deletion of per-query buckets to leveldb expiry mechanism.The PR is currently WIP: need to add code to properly enable the expiry.