basho / riak_kv

Riak Key/Value Store
Apache License 2.0
653 stars 233 forks source link

Read repair - configure to repair primary only #1844

Closed martinsumner closed 1 year ago

martinsumner commented 1 year ago

By default, the behaviour should be unchanged. however it is now configurable to read repair primary vnodes only - fallback vnodes will not be repaired on failing GETs, they will only receive new PUTs.

See schema change for more details.

https://github.com/basho/riak_test/pull/1369

martinsumner commented 1 year ago

The change from do_diffobj_put/3 to do_handoff_put/3 breaks the test https://github.com/basho/riak_test/blob/develop-3.0/tests/overlap_with_crdt_tag.erl

This is (as per usual) a bucket type issue. The original assumption of bucket types is that they should be created and activated, only when the cluster is fully formed. However, we may need to expand (join nodes) at any time.

As a node joins, the bucket types might not be propagated to the node before the handoffs start. In the case of the new do_handoff_put/3 function this means that {error, no_type} may be returned when the bucket properties are fetched for a typed bucket - and hence an attempt to treat the properties as a list will crash and the item will not handoff.

The previous do_diffobj_put also had potential issues with this - https://github.com/basho/riak_kv/blob/riak_kv-3.0.12/src/riak_kv_vnode.erl#L3649. However, the if there is already an object present for that type, it can be assumed that the node is re-joining and so has the metadata.

It is not clear that if PUTs immediately after joins could also have similar issues. This may not be exclusive to handoff.

martinsumner commented 1 year ago

Need to consider UpdateHooks as well - https://github.com/basho/riak_kv/blob/riak_kv-3.0.12/src/riak_kv_vnode.erl#L3665-L3667 - if we use the general PUT code should the reason by a PutArg, and passed through? Only impacts the deprecated Yokozuna.