khepri_adv: Add `delete_reason` prop, accumulate keep-while expirations by the-mikedavis · Pull Request #305 · rabbitmq/khepri

the-mikedavis · 2024-10-15T21:02:31Z

This change adds a delete_reason prop to the props map for nodes deleted with a function from the adv APIs khepri_adv and khepri_adv_tx. Tree nodes which are the target of deletion commands gain a delete_reason property set to direct. This change also includes any tree nodes which were deleted because of an expired keep-while condition in the node props map. These expired nodes have a delete_reason property set to keep_while instead. The delete_reason property lets callers filter on whether a change was a deletion and distinguish nodes deleted directly or indirectly.

codecov · 2024-10-15T21:08:54Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.75%. Comparing base (5e89ae2) to head (46b6952).
Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #305   +/-   ##
=======================================
  Coverage   89.75%   89.75%           
=======================================
  Files          22       22           
  Lines        3230     3241   +11     
=======================================
+ Hits         2899     2909   +10     
- Misses        331      332    +1

Flag	Coverage Δ
erlang-25	`89.01% <100.00%> (+0.03%)`	⬆️
erlang-26	`89.50% <100.00%> (+0.19%)`	⬆️
erlang-27	`89.69% <100.00%> (+<0.01%)`	⬆️
os-ubuntu-latest	`89.75% <100.00%> (+0.03%)`	⬆️
os-windows-latest	`89.66% <100.00%> (+0.18%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This change adds a new `delete_reason` key to the `node_props()` map which is either `direct` or `keep_while`. This commit changes `khepri_tree` to add the `direct` delete reason for tree nodes which were deleted because they were the target of a delete command. The `keep_while` reason will be added in the child commit. The motivation to include a delete reason is that in the child commit we will include tree nodes deleted because their keep-while conditions became unsatisfied. `delete_reason` enables the caller to distinguish between tree nodes deleted "directly" (the target of the delete command) and the others which were expired. It also enables callers of put operations to distinguish between puts (tree node creations and updates) and deletions, since a put operation could also trigger a keep-while expiration.

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.

dumbbell

Thank you!

…actions ... if the effective machine version is also old. [Why] A transaction function is extracted from the modules of a given Khepri member and is executed on all members. These members may run an older or a newer version of Khepri compared to the member that sent the transaction. Therefore, the transaction function and the Khepri public API must be aware of the effective machine version and adapt their behaviour to make sure that all state machine reaches the same state after applying a transaction. Without this, after the same transation function, an old and a new state machine may compute different state. This problem surfaced because #303 and #305 changed the return values of the advanced API: * all functions return a node props map, even those that operate on a single tree node. * all tree nodes indirectly deleted as a byproduct of another put or delete may be included in the returned node props map. This was ok with non-transaction functions because the return values are handled locally, regardless of the effective machine version. However, transactions were broken as explained in the previous paragraphs. [How] First, we introduce a new public API, `khepri_tx:does_api_comply_with()` that returns a boolean to indicate if an old or new behaviour should be expected for a specific breaking change. We then use the effective machine version passed in Ra metadata to `khepri_machine:apply()` to determine how to adapt the behaviour and return value. For queries and read-only transactions, we will need to change how we call Ra because the regular query API doesn't provide the effective machine version. This will be part of a later commit.

[Why] Since #305, the returned node props map include tree nodes that were deleted as a byproduct of another put or delete. Things like the expiration of a `keep_while` condition. [How] This was a breaking change and this new option allows the caller to restore the previous behaviour of only getting directly impacted tree nodes. This will be useful in follow-up commits to solve backward compatibility in transaction handlind when some Khepri members run older versions of Khepri.

…actions ... if the effective machine version is also old. [Why] A transaction function is extracted from the modules of a given Khepri member and is executed on all members. These members may run an older or a newer version of Khepri compared to the member that sent the transaction. Therefore, the transaction function and the Khepri public API must be aware of the effective machine version and adapt their behaviour to make sure that all state machine reaches the same state after applying a transaction. Without this, after the same transation function, an old and a new state machine may compute different state. This problem surfaced because #303 and #305 changed the return values of the advanced API: * all functions return a node props map, even those that operate on a single tree node. * all tree nodes indirectly deleted as a byproduct of another put or delete may be included in the returned node props map. This was ok with non-transaction functions because the return values are handled locally, regardless of the effective machine version. However, transactions were broken as explained in the previous paragraphs. [How] We already introduced a new public API, `khepri_tx:does_api_comply_with()` that returns a boolean to indicate if an old or new behaviour should be expected for a specific breaking change. We use this API to determine how to adapt the behaviour and return value of puts and deletes. For queries and read-only transactions, we will need to change how we call Ra because the regular query API doesn't provide the effective machine version. This will be part of a later commit.