Update AAE fullsync to support bucket types

[ian-mi]

Fixes issue #663 (RIAK-1292) (RIAK-1294) (RIAK-1295) by updating encode_obj_msg to support the w2 wire protocol, and by changing the AAE sink to use an updated decode_obj_msg function which can now properly handle bucket type property hashes.

encode_obj_msg and decode_obj_msg are not currently proper inverses of each other, but I kept the current behavior as not all messages passed to decode_obj_msg are guaranteed to be encoded using encode_obj_msg.

I have manually verified that this allows keys with a bucket type to be synced using AAE fullsync, but a test combining bucket types and AAE fullsync should probably be added to riak_test.

[bsparrow435]

Code looks good. Will add a test for this tomorrow to riak_test and then we can merge.

[bsparrow435]

Still in progress, riak_test setup has taken longer than anticipated. Will update again today.

[bsparrow435]

Test finished, fails as expected with current code but am also getting failure with this branch. Investigating more now.

[bsparrow435]

riak_test PR: basho/riak_test#770

Fails without this change, passes with the change.

👍 for merge

[bsparrow435]

@borshop 👍 bffb7df

[bsparrow435]

+1 bffb7df

[bsparrow435]

👍 bffb7df

[bsparrow435]

@borshop retry

[bsparrow435]

@borshop: retry

[bsparrow435]

@borshop :retry

[bsparrow435]

+1 bffb7df

[bsparrow435]

@borshop merge

[bsparrow435]

@borshop why have you forsaken me?

[fadushin]

Retry needs to be on the branch, not on the PR, I think.

[bsparrow435]

@jonmeredith or @lordnull i'm told by others that this needs a review from one of you all before we can re-merge. Test is basho/riak_test#770

enjoy.

[bsparrow435]

Ping @jonmeredith or @lordnull . I've been told by leads that I do not have review rights so one of you needs to review this.

[lordnull]

This particular pr has been merged, so to get it in again a new pr will need to be opened. github can't re-open merged pr's unfortunately.

Since it's been merged once, it'll likely be an easy review.

[jonmeredith]

@bsparrow435 hey - did this ever get re-opened? I'm assuming it should be fixed on 2.0.6 and 2.next ?

[bsparrow435]

I'm on vacation, @ian-mi will need to re-open. Just make a new branch Ian with these changes so we can re-merge the re-merge before reverting and then re-merging :).

[jonmeredith]

A magical dance. Sorry, didn't realize you were out - enjoy.

[cmeiklejohn]

@ian-mi Will there be a new PR opened for this that I can review?

[cmeiklejohn]

@ian-mi @bsparrow435 Will there be a new PR for this today against develop?

[bsparrow435]

Yes, Ian is working on this now. Git is complaining about his revert of the revert so he may just need to make a fresh branch and just copy/paste the changes.

[jonmeredith]

Although this is reverted and we're looking for a more general solution to replicated bucket properties, I wanted to add a note here in case we use something like this in the interim.

I can definitely see why it's logically sound to put the decoding stuff into riak_repl_util, but that means that the objects will travel between the aae_sink and the worker process as decoded objects rather than as binaries and I'm concerned this will cause a minor performance regressions - generating more garbage on the aae_sink process and increased term copying.

[ian-mi]

Thank you for the feedback. I went with the current implementation as it was the simplest and I wanted to minimize changes to the way the code is currently structured. I can investigate modifying this PR to do all decoding in the worker process if you don't already have an alternative solution in mind. I agree we should eventually unify the handling of replicated bucket properties between realtime and both fullsync methods with a more general solution.