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

Update to latest riak_ensemble integrity approach #1002

Merged
merged 6 commits into from Jul 12, 2014

Conversation

jtuple
Copy link
Contributor

@jtuple jtuple commented Jul 11, 2014

This is the sibling pull-request for basho/riak_ensemble#37. That pull-request completely changes how riak_ensemble performs data validation and integrity checking, completely removing the need for backend based trust/syncing.

This pull-request updates riak_kv to work with this new approach. For the most part, that means deleting a bunch of code that is no longer necessary. As well as updating the ensemble_status command to no longer reference AAE or trust -- since neither of those concepts apply anymore.

For more details, look at the individual commits.

_Note:_ a discussion with product management needs to happen concerning the new output of ensemble_status as well as how we should expose the new integrity checking settings to riak.conf. The old discussion about trust no longer applies since that setting has been removed. I think the approach as implemented in this pull-request is good enough for 2.0 RC But, we need to have that discussion / change things before 2.0 final.

riak_ensemble now supports an integrated integrity checking mechanism,
and therefore no longer requires nor supports backend based trust and
syncing logic. This commit removes all such logic from riak_kv.
The previous approach for consistent AAE was designed to function as the
backend syncing mechanism for K/V ensembles. Now that riak_ensemble no
longer requires backend syncing, this approach is no longer necessary
nor is it correct.

This commit completely removes the previous approach, reducing consistent
AAE to simple read repair now that riak_ensemble has a read repair mechanism.
This not only simplifies things, but this approach also properly works with
the new integrated integrity/validation logic in riak_ensemble.
This commit adds the new synctree_path callback to the K/V ensemble
backend. This callback is used to override how riak_ensemble maps
peer synctrees to on-disk LevelDB instances.

This commit implements the callback using a M:N mapping, where all
peers that correspond to a single vnode map to a common LevelDB
instance. This makes the number of synctree LevelDB instances scale
with the size of the ring, rather than the number of distinct preflists.

This is the same approach used for AAE in hashtree.erl.

Requiring fewer instances makes better use of file handles, write buffers,
and internal LevelDB caches.
Update riak_kv_ensemble_console to no longer talk about trust or AAE,
as backend based trust and AAE syncing has been completely removed.

The console output now references concepts relevant to the new
synctree approach directly integrated into riak_ensemble.

Remove the trust setting from riak_kv.schema since the setting no
longer exists. Providing schema mappings for the new relevant settings
is left for a future commit.
@jtuple
Copy link
Contributor Author

jtuple commented Jul 11, 2014

Concerning the builder failure, this pull-request will fail until the sibling riak_ensemble pull-request is merged.

@@ -386,17 +399,11 @@ print_variations() ->
ensemble_overview(Details2),
io:format("~n~n"),

%% Enabled, enough nodes, but AAE not enabled
io:format("(*** AAE disabled when required ***)~n~n"),
Details3 = Details2#details{nodes=['dev1@127.0.0.1',
Copy link
Contributor

Choose a reason for hiding this comment

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

Poor Details3 got lost in the shuffle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aye, easier to just kill it then renumber all the other lines. Keeping diff minimal/etc.

andrewjstone added a commit to basho/riak_test that referenced this pull request Jul 11, 2014
Include {error, <<"failed">>} as allowed failure so that test passes
with changes for basho/riak_ensemble#37 and basho/riak_kv#1002
andrewjstone added a commit to basho/riak_test that referenced this pull request Jul 11, 2014
Allow {error, <<"failed">>} as an error response in ensemble_sync. Fixes
the test with basho/riak_ensemble#37 and basho/riak_kv#1002
borshop added a commit that referenced this pull request Jul 11, 2014
Update to latest riak_ensemble integrity approach

Reviewed-by: andrewjstone
borshop added a commit that referenced this pull request Jul 12, 2014
Update to latest riak_ensemble integrity approach

Reviewed-by: andrewjstone
borshop added a commit that referenced this pull request Jul 12, 2014
Update to latest riak_ensemble integrity approach

Reviewed-by: andrewjstone
borshop added a commit that referenced this pull request Jul 12, 2014
Update to latest riak_ensemble integrity approach

Reviewed-by: andrewjstone
The riak_kv.consistency_trust setting has been removed, therefore this
commit updates riak_kv_schema_tests to no longer test for this setting.
borshop added a commit that referenced this pull request Jul 12, 2014
Update to latest riak_ensemble integrity approach

Reviewed-by: andrewjstone
@jtuple
Copy link
Contributor Author

jtuple commented Jul 12, 2014

@borshop merge

@borshop borshop merged commit 9984833 into develop Jul 12, 2014
@seancribbs seancribbs deleted the jdb-ensemble-integrity branch April 1, 2015 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants