Use effects to initialize new projections #255
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
the-mikedavis
force-pushed
the
md-has-projection/2-and-effect-free-register
branch
from
cb81c8b
to
b509150
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #255 +/- ##
==========================================
+ Coverage 89.34% 89.75% +0.41%
==========================================
Files 20 20
Lines 2975 2998 +23
==========================================
+ Hits 2658 2691 +33
+ Misses 317 307 -10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This function checks if a projection is currently registered. A caller
might want to use it instead of sending `#register_projection{}`
commands and relying on idempotency because `#register_projection{}`
commands contain a horus standalone function, and the horus function's
beam code can consume a non-trivial amount of data in the Ra log.
We will use the `khepri_machine:has_projection/2` helper function in
the child commit to check whether a projection exists within the
`apply/3` clause that handles `#register_projection{}` commands so that
we can check for existing projections without the side effect of
creating an ETS table.
the-mikedavis
force-pushed
the
md-has-projection/2-and-effect-free-register
branch
from
b509150
to
c3ae51f
Compare
dumbbell
requested changes
Mar 26, 2024
Previously the machine would initialize projections in-place within
`apply/3`. That would cause projections to be restored ahead of time
when the machine was recovering if it applied a `#register_projection{}`
command.
We can use an aux effect instead for the creation and initial filling
of the projection's ETS table. Aux effects are skipped during recovery.
Once the machine is fully recovered and the server enters the
`recovered` state, we emit a `restore_projections` aux effect that
properly (re)creates and fills the projections. (This is already done by
`khepri_machine`.)
This increases coverage for the projection related functions in the `khepri` API.
the-mikedavis
force-pushed
the
md-has-projection/2-and-effect-free-register
branch
from
c3ae51f
to
06712a1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
khepri_machineshouldn't initialize projections in itsapply/3callback. That function might be called by Ra when recovering and folding through the log from a snapshot. If the log contains a#register_projection{}command then Khepri will mistakenly create the ETS table and fill it with whatever is in the tree at that moment. Effects like#trigger_projection{}are discarded during recovery, so restarting a server could lead to inconsistencies in projection tables between cluster members.Instead we can use an aux effect to perform the side effects like we do already for
restore_projectionsand#trigger_projection{}effects. This will be correctly discarded when Khepri is recovering. Then projections will be correctly restored once the machine is recovered with the existingstate_enter(recovered, State)callback implementation.To make the
#register_projection{}apply/3clause pure I've added a helperkhepri_machine:has_projection/2. Exposing it askhepri:has_projection/2isn't strictly necessary for this fix but it could be a nice win to take advantage of in the server. Currently we submit#register_projection{}commands every time the broker starts and rely on idempotency. A large projection function could be a non-trivial amount of data to store in the log though, so it would be nice to skip the command if we can detect that the projection is already registered.