Skip to content

khepri_adv, khepri_tx_adv: Always return khepri_adv:many_results()#303

Merged
dumbbell merged 3 commits into
mainfrom
advanced-apis-all-return-node-props-map
Oct 11, 2024
Merged

khepri_adv, khepri_tx_adv: Always return khepri_adv:many_results()#303
dumbbell merged 3 commits into
mainfrom
advanced-apis-all-return-node-props-map

Conversation

@dumbbell
Copy link
Copy Markdown
Collaborator

@dumbbell dumbbell commented Oct 9, 2024

Why

Functions such as khepri_adv:get() and khepri_adv:put() extracted the single node's properties from the query/command return value and returned only that.

Therefore, the return value had a different form than other advanced functions' full node properties map.

In a future patch, we want to add the nodes deleted by the "expiration" of keep_while conditions. They can be deleted after a delete or a put. If we want to achieve that, we need to make all functions return the same thing.

How

All functions that returned the single node props value are simplified to return the whole map, regardless of the number of tree nodes in the map.

@dumbbell dumbbell added the enhancement New feature or request label Oct 9, 2024
@dumbbell dumbbell added this to the v0.17.0 milestone Oct 9, 2024
@dumbbell dumbbell self-assigned this Oct 9, 2024
@dumbbell dumbbell force-pushed the advanced-apis-all-return-node-props-map branch from 89e9807 to c17382b Compare October 9, 2024 16:25
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 9, 2024

Codecov Report

Attention: Patch coverage is 97.36842% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.75%. Comparing base (c3897fb) to head (efc491c).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/khepri_tx.erl 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #303      +/-   ##
==========================================
+ Coverage   89.66%   89.75%   +0.08%     
==========================================
  Files          22       22              
  Lines        3251     3230      -21     
==========================================
- Hits         2915     2899      -16     
+ Misses        336      331       -5     
Flag Coverage Δ
erlang-25 88.97% <97.36%> (+0.29%) ⬆️
erlang-26 89.65% <97.36%> (+0.21%) ⬆️
erlang-27 89.41% <97.36%> (+0.11%) ⬆️
os-ubuntu-latest 89.75% <97.36%> (+0.14%) ⬆️
os-windows-latest 89.62% <97.36%> (+0.14%) ⬆️

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.

@dumbbell dumbbell requested a review from the-mikedavis October 9, 2024 19:30
@dumbbell dumbbell marked this pull request as ready for review October 9, 2024 19:30
Copy link
Copy Markdown
Collaborator

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor typo I see in the second commit: "inser"=>"insert", otherwise I think this looks good 👍

@dumbbell dumbbell force-pushed the advanced-apis-all-return-node-props-map branch from c17382b to f71656a Compare October 11, 2024 12:35
[Why]
Functions such as `khepri_adv:get()` and `khepri_adv:put()` extracted
the single node's properties from the query/command return value and
returned only that.

Therefore, the return value had a different form than other advanced
functions' full node properties map.

In a future patch, we want to add the nodes deleted by the "expiration"
of `keep_while` conditions. They can be deleted after a delete or a put.
If we want to achieve that, we need to make all functions return the
same thing.

[How]
All functions that returned the single node props value are simplified
to return the whole map, regardless of the number of tree nodes in the
map.
[Why]
The projection needs a tuple to insert it in ETS in the end. Therefore
we can't use the `copy` method and store an atom. We need the projection
function to create that tuple.
@dumbbell dumbbell force-pushed the advanced-apis-all-return-node-props-map branch from f71656a to efc491c Compare October 11, 2024 14:48
@dumbbell
Copy link
Copy Markdown
Collaborator Author

Just a minor typo I see in the second commit: "inser"=>"insert", otherwise I think this looks good 👍

Thank you! It is fixed.

@dumbbell dumbbell merged commit 5e89ae2 into main Oct 11, 2024
@dumbbell dumbbell deleted the advanced-apis-all-return-node-props-map branch October 11, 2024 14:56
dumbbell added a commit that referenced this pull request Mar 27, 2025
…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.
dumbbell added a commit that referenced this pull request Mar 28, 2025
…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.
dumbbell added a commit that referenced this pull request Apr 1, 2025
…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.
dumbbell added a commit that referenced this pull request Apr 1, 2025
…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.
dumbbell added a commit that referenced this pull request Apr 1, 2025
…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.
dumbbell added a commit that referenced this pull request Apr 1, 2025
…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.
dumbbell added a commit that referenced this pull request Apr 2, 2025
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants