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

Add v0 of versioning to AAE hashtree locks, upgrades, and object hashing #1473

Merged
merged 24 commits into from
Sep 12, 2016

Conversation

bsparrow435
Copy link
Contributor

This PR adds version 0 of AAE hashing to riak_kv_entropy_manager, riak_kv_index_hashtree, riak_object, and the riak_kv_vnode. Specifically, we've added logic which requires getting a hashtree lock to provide an appropriate version and then our hashtree version determines what version of riak_object:hash will be used for the incoming data.

Upgrading is completed automatically for the user. The upgrade uses a combination of riak_core_capability with a new riak_kv variable object_hash_version and logic within the riak_kv_entropy manager to intelligently upgrade hashtrees once all conditions have been met. THe upgrade flow is as follows:

  1. User upgrade all nodes to 2.2.
  2. Each node locally starts checking to begin upgrade in riak_kv_entropy_manager within the management tick.
  3. IIF all partitions are built and have exchanged with all sibling partitions, we'll mark the riak_kv_entropy_manager state with a new pending_version.(need to exchange all siblings to catch any pre 2.1 data potentially suffering from kv679).
  4. Each poke on a riak_kv_index_hashtree check the pending version of the manager and if it's set, calls the vnode to upgrade it's local tree.
  5. Vnode destroys and makes a new hashtree with the upgrade atom included in the start opts.
  6. Once all upgraded hashtrees have registered with entropy_manager, flip our entropy_manager version to 0 to indicate this node has completed the upgrade for all local nodes.

Open areas for discussion:

  1. How best to handle default case of riak_object:hash without provided version(yz hashing). Current logic flips the hash when the entropy_manager notes all local trees have upgraded.
  2. hashtree:destroy only removes data within the partition dir(i.e. ./data/antie_entropy/PARTID/*). The partition ID's still need to be cleaned up out of the anti-entropy directory.

Brian Sparrow added 19 commits June 14, 2016 03:06
…atch pre 2.1 vclocks which require a rewrite to be safe

Added riak_kv boolean environment variable `hash_only_vclock` which
enables/disabled the vclock hashing logic. Additionally, when enabled,
we will no longer do the expensive compare/logging on read-repair to
catch old pre2.1 vclocks who require a rewrite.

Additional logic must be added to `yz_kv.erl` so the hashes in the
entropy_data element of the Solr documents match what is stored in KV
AAE. Upgrading the hash logic requires clearing all YZ documents and
re-indexing everything. This is because we store the obj hash in the
Solr document. This will hopefully change in the future to allow for
easier AAE changes.
Add a static version “v0” to the data_root of the riak_kv AAE
directory. This allows us to isolate old/newer trees as well as support
downgrades which will not allow old and new trees to exchange either
locally or across MDC.
New error `bad_version` added to the return from
`riak_kv_index_hashtree:do_get_lock` if the requested version and the
local version of the hash tree do not match. Requires changes in
yokozuna and riak_repl to fully function.
Change boolean configuration variable to integer version starting with
0.

Change all specs from atom() to non_neg_integer() for version number.

Add extra logging for invalid non-integer hash version configurations.
Was not correctly handling case where Result list was empty of valid
object responses or only a single valid response.
Add version 0 of object hashing(config `object_hash_version`) which
hashes only the vclock to represent a version of an object. Also add
versioning to index_hashtree get_lock API which now takes a Version
parameter. The absence of a version is assumed to be `undefined` and
will only be allowed to get a lock if the hash on disk is in legacy
format.

Upgraded hash trees are located within the anti_entropy data_dir under
the directory v0. Currently the only supported version is 0 but the
code allows for support of increasing integer versions.

Add verbose riak_object:equals check to get_core for all read
operations who do not trigger read repair. This logic is to detect
potentially pre-2.1 data which may be impacted by old vector clock bugs
and should be rewritten before upgrading the hash version for safety.
WIP commit in case of computer failure or something like that.
Also, add some testing logs and debug functions.
@@ -116,16 +124,27 @@ release_lock(Pid) ->
%% will try to acquire a concurrency lock. If successsful, the request is
%% then forwarded to the relevant index_hashtree to acquire a tree lock.
%% If both locks are acquired, the pid of the remote index_hashtree is
%% returned.
%% returned. This function assumes an undefined version of the hashtree
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of stating what the function does, I feel like this comment would be more helpful if it explained why it does it. Something like "this is left over for compatibility in mixed clusters with pre-2.2 nodes, from before we added versioning to the hashtree."

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 change.

case [RObj || {_Idx, {ok, RObj}} <- Results] of
[] ->
ok;
[_|[]] ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Very small nitpick, but the pattern I normally see to match a singleton list is just [_]. I'm sure this works just as well, but feels a little bit unidiomatic.

Brian Sparrow added 4 commits September 8, 2016 14:28
Additionally, cleaned up the anode code which killed the hash tree on
upgrade. Removed one binary layer from the riak_object:hash and added
the build locks(different from concurrency lock(doh)) to the upgrade
procedure so we won’t kill trees and then have them sit empty until we
can get a build lock.
@nickelization
Copy link
Contributor

@bsparrow435 So it occurred to me this morning that we should probably have an option to disable automatically upgrading to the new tree format. It doesn't necessarily have to be documented or supported, but I think it would be good to provide an escape hatch. If nothing else, it may make testing much easier if we want to, say, write a riak_test module that starts with 2.2 and then performs a downgrade (e.g. something like verify_riak_object_reformat).

@nickelization
Copy link
Contributor

+1 0e6bb3e

borshop added a commit that referenced this pull request Sep 12, 2016
Add v0 of versioning to AAE hashtree locks, upgrades, and object hashing

Reviewed-by: nickelization
@nickelization
Copy link
Contributor

@borshop merge

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.

3 participants