-
Notifications
You must be signed in to change notification settings - Fork 171
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
Fixed an issue with some multi_backend versions not matching regular versions #157
Conversation
Set the hash symbol to dollars
@jrwest mind a review, bolth of the problem you encountered, and the way this addresses the leveldb issue? |
@@ -0,0 +1,39 @@ | |||
%% -*- erlang -*- | |||
|
|||
%% Yo, this is just a test backend for bitcask's multi_backend schema. It's not real |
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.
so we would need this in each backend repo? I think I see why but if we change the multi_backend don't we need to change it in 3 other places now...all to prevent changing the multi_backend bitcask/leveldb schemas in 2 places?
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.
i suppose the schemas for the latter case change more often (shrug).
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.
so, this is a contrived test schema for the minimum I need just to prove the multi version of the bitcask schema is working correctly. We will need one of these in the eleveldb repo, but not for memory because memory lives in the riak_kv repo, which has access to the actual multi_backend.schema. This thing shouldn't ever have to change, unless we drastically change how multi_backend is handled, in which case, this test won't be valid anyway.
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.
yea its probably the best of the less ideal cases. Unfortunately, the test also won't fail when its invalid :/
To actually fix the issue I ran into this morning a change in Riak [1] [2] and Riak EE is also necessary. [1] basho/riak@66666e4 |
@jrwest I'll open a PR on those two repos once the same thing is done for leveldb |
+1 merge after the whitespace fixes. fixes bolth the issue I ran into with the improper schema (assuming the Riak build has been modified as described above several times in this PR) and it addresses basho/eleveldb#80 as best it can for now |
versions Also added a test that will pick problems like that up going forward.
…chema Fixed an issue with some multi_backend versions not matching regular versions
Also added a test that will pick problems like that up going forward.
Added a way to include bitcask_multi.schema a la carte. Will have to modify riak and riak_ee to use this, but is a decent compromise for the issue described in basho/eleveldb#80