Add breadth-first AAE exchange#628
Conversation
This commit ports the breath-first exchange algorithm from the synctree code in riak_ensemble to the hashtree code here in riak_core. This level-by-level exchange is necessary to support streaming and/or pipelined AAE exchange -- an approach that will soon underpin the new AAE-based fullsync replication protocol in Riak.
There was a problem hiding this comment.
Why have Diffs, why not do the same as exchange_final?
There was a problem hiding this comment.
Good observation. riak_ensemble_util:orddict_delta/2 returns [ {'$none', any()} | {any(), '$none'} | {any(), any()} ]. Thus, if the local cluster has some missing sub-buckets or segments as compared to the remote cluster; the result of this function (line below) would include some '$none's. How does the next level in the recursion handle that?
There was a problem hiding this comment.
I just meant there was an unnecessary variable being set, it looks like the exchange_level/6 usage strips the difference on L842 and knows that it needs to recurse to the next level if there are any differences.
|
Ran coverage tests and confirmed compare2 was covered by EQC. I did notice that there was a local Not critical for merge, but if left as it is should add a comment saying what the plan for it is. |
|
I suppose we have lost test coverage for |
There was a problem hiding this comment.
What I mean is that maybe this line should read
[BK || {BK, _} <- Diffs, BK =/= '$none']There was a problem hiding this comment.
BK is the key in the orddict, so I think it will be keyed as Bucket. Here's an example of orddict_delta showing how the key is separate from the '$none' so you can see BK will not be set to it.
(riak@127.0.0.1)87> riak_ensemble_util:orddict_delta(orddict:from_list([{a,1},{b,2},{c,3}]),orddict:from_list([{b,2},{c,5},{d,4}])).
[{a,{1,'$none'}},{c,{3,5}},{d,{'$none',4}}]
|
Code inspection looks good - only issues are minor. Will complete other PR reviews before giving final approval |
|
+1 to merge - but this should be against 2.0 so not telling bors to do it. Ran with this and sibling branches and got through riak_tests. |
This commit ports the breath-first exchange algorithm from the synctree code in riak_ensemble to the hashtree code here in riak_core. This level-by-level exchange is necessary to support streaming and/or pipelined AAE exchange -- an approach that will soon underpin the new AAE-based fullsync replication protocol in Riak.
|
@jtuple I think this should be closed against develop and just merge the 2.0 branch in. Can you confirm? |
4 participants
This commit ports the breath-first exchange algorithm from the synctree code in riak_ensemble to the hashtree code here in riak_core. This level-by-level exchange is necessary to support streaming and/or pipelined AAE exchange -- an approach that underpins the new AAE-based fullsync replication protocol in Riak Enterprise.
Sibling pull-requests: basho/riak_kv#1023, basho/riak_repl#616