Always pass exported functions to Ra queries #242
Conversation
[Why] We were already making sure that the path was native and valid but we were not using it in the actual query… We were using the original argument instead.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #242 +/- ##
==========================================
+ Coverage 88.74% 89.37% +0.63%
==========================================
Files 20 21 +1
Lines 2896 3079 +183
==========================================
+ Hits 2570 2752 +182
- Misses 326 327 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
dumbbell
force-pushed
the
replace-funs-by-mfa-in-ra-queries
branch
6 times, most recently
from
55396b7 to
bc0a556
Compare
[Why] Using `fun()` is fragile if it has to be executed on a remote node. Indeed, the remote node may have another version of the module hosting the function and the function reference may be invalid. [How] Now, we always use an exported function. Callers of any folding APIs should do the same. Fixes #238.
[Why]
Using `fun()` is fragile if it has to be executed on a remote node.
Indeed, the remote node may have another version of the module hosting
the function and the function reference may be invalid.
[How]
In the previous commit, Khepri stopped using fun references internally
where we know they can be executed on a remote node.
In this commit, we ensure that we apply the same constraint to external
functions passed to fold/foreach/map/filter queries and read-only
transactions.
If the caller passes such a function as a function reference instead of
the MFA tuple, Khepri will throw an exception to indicate this is not
permitted.
Here is an example for those queries and R/O transactions:
khepri:fold(
StoreId,
PathPattern
{my_mod, my_fold_func, [SomeState]},
InitialAcc).
Where `my_mod:my_fold_func()` looks like:
my_fold_func(SomeState, Path, NodeProps, Acc) ->
...
NewAcc.
dumbbell
force-pushed
the
replace-funs-by-mfa-in-ra-queries
branch
from
bc0a556 to
f02a92b
Compare
|
The patch is finished but I would like to test it with RabbitMQ before merging it. |
|
I have second thoughts on the fact that MFAs are good enough as a solution to execute code remotely. Yes, with the current patch, we ensure that a query won’t break because the function reference is invalid on another node. But that doesn’t mean that the function exported remotely is the same as the local one. This is quite unintuitive. Thinking out loud here; if a leader query was requested, what about we check the cursor on the leader, wait for the local cursor to catch up, then execute the query function locally? Don’t we have the same consistency guaranty? It may take more time to answer the query though, because the catch up can be longer than a remote execution. |
|
This pull request was superseded by #260. |
Why
Using
fun()is fragile if it has to be executed on a remote node. Indeed, the remote node may have another version of the module hosting the function and the function reference may be invalid.How
There are two parts:
fun()are only accepted for local queries.Fixes #238.