Skip to content

Commit

Permalink
khepri_tree: Accumulate keep-while expirations
Browse files Browse the repository at this point in the history
With this change any tree nodes removed from the tree because their
keep-while condition became unsatisfied are accumulated in the return
value of the put or delete operation which caused their deletion. These
expired tree nodes have props maps in the return map with the
`delete_reason` key set to `keep_while`. This enables callers of delete
operations to distinguish between deletes which were the target of the
delete command (`direct`) and expired nodes (`keep_while`). This also
enables callers of put operations to distinguish between creates or
updates and deletions.

This change only affects functions which return the
`khepri_adv:node_props_map()` - functions from the `khepri_adv` and
`khepri_adv_tx` modules. Previously the node props map did not include
the tree nodes removed because of expired keep-while conditions.
  • Loading branch information
the-mikedavis committed Oct 15, 2024
1 parent f995b55 commit d3411b0
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 32 deletions.
65 changes: 46 additions & 19 deletions src/khepri_adv.erl
Original file line number Diff line number Diff line change
Expand Up @@ -332,11 +332,17 @@ put(StoreId, PathPattern, Data) ->
%% in the path pattern is not met, an error is returned and the tree structure
%% is not modified.
%%
%% The returned `{ok, NodeProps}' tuple contains a map with the properties and
%% payload (if any) of the targeted tree node: the payload was the one before
%% the update, other properties like the payload version correspond to the
%% updated node. If the targeted tree node didn't exist, `NodeProps' will be
%% an empty map.
%% The returned `{ok, NodePropsMap}' tuple contains a map where keys correspond
%% to the path to a node affected by the put operation. Each key points to a
%% map containing the properties and prior payload (if any) of a tree node
%% created, updated or deleted by the put operation. If the put results in the
%% creation of a tree node this props map will be empty. If the put updates an
%% existing tree node then the props map will contain the properties of the
%% tree node before the update. The `NodePropsMap' map might also contain
%% deletions if the put operation leads to an existing tree node's keep-while
%% condition becoming unsatisfied. The props map for any nodes deleted because
%% of an expired keep-while condition will contain a `delete_reason' key set to
%% `keep_while'.
%%
%% The payload must be one of the following form:
%% <ul>
Expand Down Expand Up @@ -452,11 +458,17 @@ put_many(StoreId, PathPattern, Data) ->
%% in the path pattern is not met, an error is returned and the tree structure
%% is not modified.
%%
%% The returned `{ok, NodePropsMap}' tuple contains a map where keys
%% correspond to the path to a tree node matching the path pattern. Each key
%% then points to a map containing the properties and payload (if any) of the
%% targeted tree node: the payload was the one before the update, other
%% properties like the payload version correspond to the updated node.
%% The returned `{ok, NodePropsMap}' tuple contains a map where keys correspond
%% to the path to a node affected by the put operation. Each key points to a
%% map containing the properties and prior payload (if any) of a tree node
%% created, updated or deleted by the put operation. If the put results in the
%% creation of a tree node this props map will be empty. If the put updates an
%% existing tree node then the props map will contain the properties of the
%% tree node before the update. The `NodePropsMap' map might also contain
%% deletions if the put operation leads to an existing tree node's keep-while
%% condition becoming unsatisfied. The props map for any nodes deleted because
%% of an expired keep-while condition will contain a `delete_reason' key set to
%% `keep_while'.
%%
%% The payload must be one of the following form:
%% <ul>
Expand Down Expand Up @@ -831,9 +843,15 @@ delete(PathPattern, Options) when is_map(Options) ->
%% one tree node would match at the time. If you want to delete multiple nodes
%% at once, use {@link delete_many/3}.
%%
%% The returned `{ok, NodeProps}' tuple contains a map with the properties and
%% payload (if any) of the targeted tree node as they were before the delete.
%% If the targeted tree node didn't exist, `NodeProps' will be an empty map.
%% The returned `{ok, NodePropsMap}' tuple contains a map where keys
%% correspond to the path to a deleted tree node. Each key then points to a
%% map containing the properties and payload (if any) of that deleted tree
%% node as they were before the delete. Tree nodes in this map which were
%% the target of the delete command will have a `delete_reason' key set to
%% `direct' in their associated props map. Any tree nodes deleted because their
%% keep-while condition became unsatisfied due to the deletion will have the
%% `delete_reason' key set to `keep_while' instead. (See {@link
%% khepri_condition:keep_while()}.)
%%
%% When doing an asynchronous update, the {@link handle_async_ret/1}
%% function should be used to handle the message received from Ra.
Expand All @@ -842,7 +860,8 @@ delete(PathPattern, Options) when is_map(Options) ->
%% ```
%% %% Delete the tree node at `/:foo/:bar'.
%% {ok, #{data := value,
%% payload_version := 1}} = khepri_adv:delete(StoreId, [foo, bar]).
%% payload_version := 1,
%% delete_reason := direct}} = khepri_adv:delete(StoreId, [foo, bar]).
%% '''
%%
%% @param StoreId the name of the Khepri store.
Expand Down Expand Up @@ -925,18 +944,26 @@ delete_many(PathPattern, Options) when is_map(Options) ->
%% The returned `{ok, NodePropsMap}' tuple contains a map where keys
%% correspond to the path to a deleted tree node. Each key then points to a
%% map containing the properties and payload (if any) of that deleted tree
%% node as they were before the delete.
%% node as they were before the delete. Tree nodes in this map which were
%% the target of the delete command will have a `delete_reason' key set to
%% `direct' in their associated props map. Any tree nodes deleted because their
%% keep-while condition became unsatisfied due to the deletion will have the
%% `delete_reason' key set to `keep_while' instead. (See {@link
%% khepri_condition:keep_while()}.)
%%
%% When doing an asynchronous update, the {@link handle_async_ret/1}
%% function should be used to handle the message received from Ra.
%%
%% Example:
%% ```
%% %% Delete the tree node at `/:foo/:bar'.
%% %% Delete all tree nodes matching `/*/:bar'.
%% {ok, #{[foo, bar] := #{data := value,
%% payload_version := 1},
%% [baz, bar] := #{payload_version := 1}}} = khepri_adv:delete_many(
%% StoreId, [foo, bar]).
%% payload_version := 1,
%% delete_reason := direct},
%% [baz, bar] := #{payload_version := 1,
%% delete_reason := direct}}} =
%% khepri_adv:delete_many(
%% StoreId, [?KHEPRI_WILDCARD_STAR, bar]).
%% '''
%%
%% @param StoreId the name of the Khepri store.
Expand Down
31 changes: 25 additions & 6 deletions src/khepri_tree.erl
Original file line number Diff line number Diff line change
Expand Up @@ -1328,7 +1328,9 @@ handle_keep_while_for_parent_update(
handle_keep_while_for_parent_update(
#walk{tree = Tree,
node = ParentNode,
reversed_path = ReversedPath} = Walk,
reversed_path = ReversedPath,
tree_options = TreeOptions,
fun_acc = Acc} = Walk,
AppliedChangesAcc) ->
ParentPath = lists:reverse(ReversedPath),
IsMet = is_keep_while_condition_met_on_self(
Expand All @@ -1341,7 +1343,11 @@ handle_keep_while_for_parent_update(
%% This parent node must be removed because it doesn't meet its
%% own keep_while condition. keep_while conditions for nodes
%% depending on this one will be evaluated with the recursion.
Walk1 = Walk#walk{node = delete},
{ok, delete, Acc1} = delete_matching_nodes_cb(
ParentPath, ParentNode,
TreeOptions, keep_while, Acc),
Walk1 = Walk#walk{node = delete,
fun_acc = Acc1},
walk_back_up_the_tree(Walk1, AppliedChangesAcc)
end.

Expand Down Expand Up @@ -1525,14 +1531,27 @@ remove_expired_nodes([], Walk) ->
{ok, Walk};
remove_expired_nodes(
[PathToDelete | Rest],
#walk{tree = Tree, applied_changes = AppliedChanges} = Walk) ->
case delete_matching_nodes(Tree, PathToDelete, AppliedChanges, #{}) of
{ok, Tree1, AppliedChanges1, _Acc} ->
#walk{tree = Tree,
applied_changes = AppliedChanges,
tree_options = TreeOptions,
fun_acc = Acc} = Walk) ->
%% See `delete_matching_nodes/4'. This is the same except that the
%% accumulator is passed through and the `DeleteReason' is provided as
%% `keep_while'.
Fun = fun(Path, Node, Result) ->
delete_matching_nodes_cb(
Path, Node, TreeOptions, keep_while, Result)
end,
Result = walk_down_the_tree(
Tree, PathToDelete, TreeOptions, AppliedChanges, Fun, Acc),
case Result of
{ok, Tree1, AppliedChanges1, Acc1} ->
AppliedChanges2 = merge_applied_changes(
AppliedChanges, AppliedChanges1),
Walk1 = Walk#walk{tree = Tree1,
node = Tree1#tree.root,
applied_changes = AppliedChanges2},
applied_changes = AppliedChanges2,
fun_acc = Acc1},
remove_expired_nodes(Rest, Walk1)
end.

Expand Down
10 changes: 9 additions & 1 deletion test/delete_command.erl
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,15 @@ delete_a_node_deep_into_the_tree_test() ->
?assertEqual({ok, #{[foo, bar, baz] => #{payload_version => 1,
child_list_version => 1,
child_list_length => 1,
delete_reason => direct}}}, Ret),
delete_reason => direct},
[foo, bar] => #{payload_version => 1,
child_list_version => 2,
child_list_length => 0,
delete_reason => keep_while},
[foo] => #{payload_version => 1,
child_list_version => 2,
child_list_length => 0,
delete_reason => keep_while}}}, Ret),
?assertEqual([], SE).

delete_existing_node_with_condition_true_test() ->
Expand Down
32 changes: 26 additions & 6 deletions test/keep_while_conditions.erl
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ keep_while_now_false_after_command_test() ->
#node{props = ?INIT_NODE_PROPS,
payload = khepri_payload:data(bar_value)}}}}},
Root),
?assertEqual({ok, #{[foo, bar] => #{}}}, Ret),
?assertEqual({ok, #{[foo, bar] => #{},
[baz] => #{delete_reason => keep_while}}}, Ret),
?assertEqual([], SE).

recursive_automatic_cleanup_test() ->
Expand Down Expand Up @@ -293,7 +294,17 @@ recursive_automatic_cleanup_test() ->
payload_version => 1,
child_list_version => 1,
child_list_length => 0,
delete_reason => direct}}}, Ret),
delete_reason => direct},
[foo, bar] => #{data => bar_value,
payload_version => 1,
child_list_version => 3,
child_list_length => 0,
delete_reason => keep_while},
[foo] => #{data => foo_value,
payload_version => 1,
child_list_version => 3,
child_list_length => 0,
delete_reason => keep_while}}}, Ret),
?assertEqual([], SE).

keep_while_now_false_after_delete_command_test() ->
Expand Down Expand Up @@ -324,7 +335,12 @@ keep_while_now_false_after_delete_command_test() ->
payload_version => 1,
child_list_version => 1,
child_list_length => 0,
delete_reason => direct}}}, Ret),
delete_reason => direct},
[baz] => #{data => baz_value,
payload_version => 1,
child_list_version => 1,
child_list_length => 0,
delete_reason => keep_while}}}, Ret),
?assertEqual([], SE).

automatic_reclaim_of_useless_nodes_works_test() ->
Expand All @@ -342,7 +358,9 @@ automatic_reclaim_of_useless_nodes_works_test() ->
child_list_version => 3},
child_nodes = #{}},
Root),
?assertEqual({ok, #{[foo, bar, baz] => #{delete_reason => direct}}}, Ret),
?assertEqual({ok, #{[foo, bar, baz] => #{delete_reason => direct},
[foo, bar] => #{delete_reason => keep_while},
[foo] => #{delete_reason => keep_while}}}, Ret),
?assertEqual([], SE).

automatic_reclaim_keeps_relevant_nodes_1_test() ->
Expand Down Expand Up @@ -370,7 +388,8 @@ automatic_reclaim_keeps_relevant_nodes_1_test() ->
payload = khepri_payload:data(relevant),
child_nodes = #{}}}},
Root),
?assertEqual({ok, #{[foo, bar, baz] => #{delete_reason => direct}}}, Ret),
?assertEqual({ok, #{[foo, bar, baz] => #{delete_reason => direct},
[foo, bar] => #{delete_reason => keep_while}}}, Ret),
?assertEqual([], SE).

automatic_reclaim_keeps_relevant_nodes_2_test() ->
Expand Down Expand Up @@ -404,6 +423,7 @@ automatic_reclaim_keeps_relevant_nodes_2_test() ->
child_nodes = #{}}}}}},
Root),
?assertEqual(
{ok, #{[foo, bar, baz, qux] => #{delete_reason => direct}}},
{ok, #{[foo, bar, baz, qux] => #{delete_reason => direct},
[foo, bar, baz] => #{delete_reason => keep_while}}},
Ret),
?assertEqual([], SE).

0 comments on commit d3411b0

Please sign in to comment.