Replies: 9 comments 16 replies
-
|
One possibility:
|
Beta Was this translation helpful? Give feedback.
-
|
See my other comment I think we just need a new reconciliation mechanism, not a new storage method, as eleveldb is NOT currently used for storage of metadata. i.e. we introduce a new reconciliation mechanism, and a capability. When the capability to support the new mechanism is available throughout the cluster - we switch from AAE-based reconciliation to the new version. In 3.4 (or later) there would then be no hashtree for metadata (and perhaps hashtree at all). |
Beta Was this translation helpful? Give feedback.
-
|
I think it is stored in DETS. The use of eleveldb is as the backend for hashtree - and currently metadata reconciliation depends on hashtree. |
Beta Was this translation helpful? Give feedback.
-
|
I think it can be in something much simpler than DETS that can be gossiped around as a single term. -module(riak_core_cluster_metadata).
-export([
get_value/2,
set_value/2
]).
-export_type([
cluster_metadata/0
]).
-record(cluster_metadata, {
schema :: pos_integer(),
generation :: vclock(),
data :: map()
}).
-opaque cluster_metadata() :: #cluster_metadata{}.
-spec get_value(Key :: term(), CM :: cluster_metadata())
-> {ok, term()} | false.
get_value(Key, #cluster_metadata{data = Data})
when erlang:is_map(Data), erlang:is_map_key(Key, Data) ->
{ok, maps:get(Key, Data)};
get_value(Key, #cluster_metadata{data = Data})
when erlang:is_map(Data) ->
false;
get_value(Key, CM) ->
erlang:error(badarg, [Key, CM]).
-spec set_value(Key :: term(), Val :: term(), CM :: cluster_metadata())
-> {ok, NewCM :: cluster_metadata()} | {error, term()}.
set_value(Key, Val, #cluster_metadata{data = Data} = CM)
when erlang:is_map(Data),
erlang:is_map_key(Key, Data),
erlang:map_get(Key, Data) =:= Val ->
%% No change
{ok, CM};
set_value(Key, Val, #cluster_metadata{generation = VC, data = Data} = CM)
when erlang:is_map(Data) ->
NewData = Data#{Key => Val},
NewCM = CM#cluster_metadata{
generation = vclock:increment(Node, VC), data = NewData},
propagate(NewCM);
set_value(Key, Val, CM) ->
erlang:error(badarg, [Key, Val, CM]).
-spec propagate(CM :: cluster_metadata())
-> {ok, NewCM :: cluster_metadata()} | {error, term()}.
%% @doc Propagates the provided CM around the cluster, returning
%% the agreed-upon latest version.
propagate(CM) ->
BinCM = externalize(CM)
%% send it around
case agreed_upon_version(BinCM) of
{ok, BinRecv} ->
{ok, internalize(BinRecv)};
Error ->
Error
end.
-spec externalize(CM :: cluster_metadata()) -> binary().
%% @doc Returns CM in a format suitable for file storage or propagation
%% around the cluster.
externalize(CM) ->
erlang:term_to_binary(CM, [compressed, {minor_version, 2}]).
-spec internalize(BinCM :: binary())
-> cluster_metadata() | {error, term()}.
internalize(BinCM) ->
BinSz = erlang:byte_size(BinCM),
{CM, Used} = erlang:binary_to_term(BinCM, [used]),
case Used of
BinSz ->
CM;
_ ->
{error, size_mismatch}
end. |
Beta Was this translation helpful? Give feedback.
-
|
So the current design, as I understand it:
From a migration perspective, it is a lot easier just to replace the hashtree backend, as the current code will handle the hastree backend not being present (i.e. in next version you point at a new hahstree backend in a new location, during migration it sees the backend isn't there and so calculates a new hashtree). However, I would agree that it is all over-engineered, and the metadata could just be a map.
The migration is more complicated here though. Having a riak_core_capability defined would help, but I think it requires a bit mroe thought beyond that. |
Beta Was this translation helpful? Give feedback.
-
|
Just to clarify my "all over-engineered" comment. I think it would be fairer to say that it is engineered to cope with a large volume of metadata - and that commonly we don't see such a large volume. However, it may be in the future that there will situations where a large volume of metadata is required. |
Beta Was this translation helpful? Give feedback.
-
|
But what constitutes a "large volume" of metadata? |
Beta Was this translation helpful? Give feedback.
-
|
Isn't the design effort really about managing how much has to be sent over the network, rather than held in memory (in that the current implementation holds all metadata in ETS table)? That is to say, the use of the riak_core_broadcast behaviour - which drives some of the requirements of how metadata is implemented. The purpose of riak_core_broadcast is if there is a large volume of metadata, and a large number of nodes - the inter-node traffic required to coordinate is minmimised. |
Beta Was this translation helpful? Give feedback.
-
|
I assume there's a good reason not to simply swap eleveldb for leveled? |
Beta Was this translation helpful? Give feedback.
-
|
Linking with previous discussion - #2 |
Beta Was this translation helpful? Give feedback.
-
Not only will the cluster's bucket-type have an All this to say, do we want to keep the behavior the same? As you know, my preference is that we just support bucket-types in configs and they're ready at start-up with the ability to reload config during runtime. I do not like the opaqueness of having a bucket-types configured at run-time. But I can create another discussion for that since that's a much larger change in terms of implementation and product feature. |
Beta Was this translation helpful? Give feedback.
-
|
The mechanism of updating properties with sending JSON via the CLI has a history of associated escaping issues when ever we've gone through major refactoring of the scripts. However, I'm not sure how easy a cuttlefish-based configuration of bucket types would be. There are also different types of properties to consider. Compare Finding the right way of setting the properties of bucket types, is complicated by the differences in the impact of dynamically changing different properties. |
Beta Was this translation helpful? Give feedback.
-
|
This is the documented list of bucket properties, which doesn't include any new ones added since 2.2.3: {
"props": {
"allow_mult": false,
"basic_quorum": false,
"big_vclock": 50,
"chash_keyfun": {
"fun": "chash_std_keyfun",
"mod": "riak_core_util"
},
"dvv_enabled": false,
"dw": "quorum",
"last_write_wins": false,
"linkfun": {
"fun": "mapreduce_linkfun",
"mod": "riak_kv_wm_link_walker"
},
"n_val": 3,
"notfound_ok": true,
"old_vclock": 86400,
"postcommit": [],
"pr": 0,
"precommit": [],
"pw": 0,
"r": "quorum",
"rw": "quorum",
"small_vclock": 50,
"w": "quorum",
"young_vclock": 20
}
}I would probably argue, the only properties that should be configurable via bucket types are: {
"props": {
"allow_mult": false,
"notfound_ok": true,
"postcommit": [],
"pr": 0,
"precommit": [],
"pw": 0,
"node_confirms": 0,
"sync_on_write": "backend"
}
} |
Beta Was this translation helpful? Give feedback.
-
|
The metadata exchange during the join I think is necessary to avoid situations where a join completes without metadata having been exchanged. However, I agree, it doesn't seem right that metadata on the joining node can overwrite cluster metadata without the plan and commit. This particular exchange function I don't think LWW is ever used when merging bucket type information, in that all conflicts should have a deterministic resolution: riak_core/src/riak_core_bucket_props.erl Lines 142 to 192 in 1728625 I think, that during an exchange conflicting information will grow the dvvset (i.e. add siblings), but on reading the bucket type from the metadata the resolver will use You said the bucket type can be deactivated in your example. I'm not sure why that would be. Could you point me in the right direction? [EDIT] The above is wrong. The resolver is used when merging bucket properties, but not bucket type properties: riak_core/src/riak_core_claimant.erl Lines 237 to 241 in 1728625 So when getting the bucket properties for a typed bucket, we get the properties for the type (resolved via LWW), and then for the specific bucket (resolved via |
Beta Was this translation helpful? Give feedback.
-
|
@aef- is the issue you were referring to caused by the fact that the [EDIT] See edit on previous comment, I was wrong about the use of the resolver function. [EDIT 2] So I think I now understand the problem you were pointing out:
|
Beta Was this translation helpful? Give feedback.
-
|
I've looked a bit again at riak_core_broadcast. The behaviour is defined in the %% Return a two-tuple of message id and payload from a given broadcast
-callback broadcast_data(any()) -> {any(), any()}.
%% Given the message id and payload, merge the message in the local state.
%% If the message has already been received return `false', otherwise return `true'
-callback merge(any(), any()) -> boolean().
%% Return true if the message (given the message id) has already been received.
%% `false' otherwise
-callback is_stale(any()) -> boolean().
%% Return the message associated with the given message id. In some cases a message
%% has already been sent with information that subsumes the message associated with the given
%% message id. In this case, `stale' is returned.
-callback graft(any()) -> stale | {ok, any()} | {error, any()}.
%% Trigger an exchange between the local handler and the handler on the given node.
%% How the exchange is performed is not defined but it should be performed as a background
%% process and ensure that it delivers any messages missing on either the local or remote node.
%% The exchange does not need to account for messages in-flight when it is started or broadcast
%% during its operation. These can be taken care of in future exchanges.
-callback exchange(node()) -> {ok, pid()} | {error, term()}.The There is nothing inherent within There is then also the question of do we want to keep using For the ring broadcasting/gossipping the ring, For the purpose of metadata exchange is the relatively simple approach of |
Beta Was this translation helpful? Give feedback.
-
|
Some more details on the broadcast mechanism. It, like
The selection is potentially cyclic, the initial tree has no inherent protection to prevent messages looping. The node makes it leaves its "eager" set, and then picks another random node to be in its "lazy" set. When a broadcast message is received to be forwarded, the broadcast is forwarded to the eager set. All nodes in the eager set that receive the Pushes to the lazy set are scheduled, and sent first as a Nodes that go down for extended periods end up as members of multiple lazy sets - and catch-up via scheduled One important detail is the generation of MessageID. The MessageID is generated by the |
Beta Was this translation helpful? Give feedback.
-
|
There is some additional confusing complexity to the above. When the peers are first initialised by calling The fact that each eager set will start with the riak_core/src/riak_core_broadcast.erl Lines 544 to 564 in 1728625 |
Beta Was this translation helpful? Give feedback.
-
|
An exchange is triggered every tick (default 10s) on each node. The exchange is prompted between with a riak_core/src/riak_core_broadcast.erl Lines 402 to 403 in 1728625 It is confusing why a riak_core/src/riak_core_broadcast.erl Lines 473 to 483 in 1728625 The exchange is prompted between this node and the |
Beta Was this translation helpful? Give feedback.
-
|
An exchange is also triggered as part of the Line 165 in 1728625 This is a separate function to the |
Beta Was this translation helpful? Give feedback.
-
|
Following the virtual meet-up on 9th April 2025, the rough plan is.
Some points that were raised:
|
Beta Was this translation helpful? Give feedback.
-
|
Tasks required for first stage (i.e. not including change to metadata definition method).
Probably needs a design decision whether to always do a full exchange, or do some validation (maybe by hash) first to see if the exchange can be bypassed. I don't think there are any eqc/eunit tests for metadata management - but there are two riak_test tests (
Changes that are locally relevant only to a node wrt metadata_management (i.e. do not depend on capability). e.g. promoting the metadata to a map with persistent_term, or some other mechanism to make looking up properties more efficient. Also, potentially change persistence method from DETS to a flat file.
Probably replace with synctree_leveled (leveled in head_only mode as with parallel-mode tictacaae). Assume existing test coverage in
Take out of
In such a way that it should be easy to build a bespoke riak with the dep added back in. |
Beta Was this translation helpful? Give feedback.
-
|
(3) is addressed on branch https://github.com/OpenRiak/riak_ensemble/tree/wday/updates-3.4-wip |
Beta Was this translation helpful? Give feedback.
-
|
Can we make sure that the logs contain lines for when new metadata is loaded showing conflicts with the local stored metadata, the source of the winning metadata and what the metadata that won was? Specifically, I am thinking of conflicting config files on different nodes and being able to track down and synchronise them. For no changes, having a single line confirming that: For new metadata, having a single line per metadata item with the new item's details: For updated metadata, having a single [warn] line per metadata item with the updated item's changes: On the node that actually has the changed config, make it clear it's from the config file or from the CLI: |
Beta Was this translation helpful? Give feedback.
-
|
Updated following the online discussion on 16/04/25. ScopeThe scope of the change was discussed. In essence two options:
The assumption is we will do (1), but there is still an opportunity to offer feedback in the next couple of days. The main drivers for choosing (1) were ease of transition, and potential "Chesterton Fence" issues with a bigger rewrite (particularly needing to know more about security metadata). Halfway-house options are complicated, and not desirable: e.g. hard to remove dvvset without impacting the use of dvv context in the messageID definition for Requirements for exchange v2Some requirements were stated for a v2 exchange:
Transition of bucket metadataThere was discussion about whether the existence of Nothing agreed about this, or whether there need to be some action on documenting/simplifying how to use Resolving sibling metadataThere is no action required on the use of LWW rather than the Replicating bucket propertiesAcknowledgement that not replicating has operational issues, but replicating also has an impact (in particular with regards to reconciling conflicting values, and also perhaps deliberately running different values for backup clusters). No action to be taken for now. Simplifying propertiesThe main candidate put forward for a bucket property to be removed due to hidden complexity was Setting properties via configurationA non-trivial decision, and so discussion postponed for another day. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
As of OpenRiak 3.2.5, cluster metadata is stored in
eleveldb, an increasingly untenable situation as the code ages.So, how do we migrate to a new storage mechanism, and what should that mechanism be?
Beta Was this translation helpful? Give feedback.
All reactions