Skip to content

Handle deleted projections tables while triggering projections#293

Merged
dumbbell merged 1 commit into
mainfrom
md/projections/handle-undefined-tables
Sep 11, 2024
Merged

Handle deleted projections tables while triggering projections#293
dumbbell merged 1 commit into
mainfrom
md/projections/handle-undefined-tables

Conversation

@the-mikedavis
Copy link
Copy Markdown
Collaborator

The test case in this change shows an example that can cause an error log. One client of Khepri sends an unregister_projections command while another sends many changes which affect a projection which is being unregistered. Using async commands makes the error much more likely to occur.

Ra applies committed commands in a batch and then handles all effects created by the batch. In this scenario the unregister_projections command may end up being applied by Ra in a batch with the async commands and the resulting effects list will contain aux effects to trigger projections (casued by the async commands) and an aux effect to unregister the projection. Some trigger aux effects may come later in the list than the unregister_projections effect and so their projection table will be deleted. In this case we want to perform a no-op for the trigger effect.

We already no-op these effects because we wrap ETS calls in try expressions. This change avoids an error log caused by ETS calls attempting to insert or delete records in an undefined table.

Example error log...

This is from the commontest output of the added test case before the fix in khepri_projection.erl:

2024-09-10T09:48:20.686591-04:00 <0.173.0>: Failed to insert record into ETS table for projection: cluster_SUITE
  Record:
    {[key],116}
  Crash:
    exception error: bad argument
     in function  ets:insert/2
        called as ets:insert(undefined,{[key],116})
        *** argument 1: the table identifier does not refer to an existing ETS table
     in call from khepri_projection:try_ets_insert/3 (/nix/persist/home/michael/src/rabbitmq/khepri/src/khepri_projection.erl, line 446)
     in call from khepri_projection:trigger_simple_projection/6 (/nix/persist/home/michael/src/rabbitmq/khepri/src/khepri_projection.erl, line 405)
     in call from khepri_machine:handle_aux/6 (/nix/persist/home/michael/src/rabbitmq/khepri/src/khepri_machine.erl, line 1242)
     in call from ra_server:handle_aux/4 (/nix/persist/home/michael/src/rabbitmq/khepri/_build/default/lib/ra/src/ra_server.erl, line 1632)
     in call from ra_server_proc:handle_effect/5 (/nix/persist/home/michael/src/rabbitmq/khepri/_build/default/lib/ra/src/ra_server_proc.erl, line 1394)
     in call from lists:foldl_1/3 (lists.erl, line 1599)
     in call from ra_server_proc:handle_effects/5 (/nix/persist/home/michael/src/rabbitmq/khepri/_build/default/lib/ra/src/ra_server_proc.erl, line 1301)

@the-mikedavis the-mikedavis added this to the v0.16.0 milestone Sep 10, 2024
@the-mikedavis the-mikedavis self-assigned this Sep 10, 2024
The test case in this change shows an example that can cause an error
log. One client of Khepri sends an `unregister_projections` command
while another sends many changes which affect a projection which is
being unregistered. Using async commands makes the error much more
likely to occur.

Ra applies committed commands in a batch and then handles all effects
created by the batch. In this scenario the `unregister_projections`
command may end up being applied by Ra in a batch with the async
commands and the resulting effects list will contain aux effects to
trigger projections (casued by the async commands) and an aux effect
to unregister the projection. Some trigger aux effects may come later in
the list than the `unregister_projections` effect and so their
projection table will be deleted. In this case we want to perform a
no-op for the trigger effect.

We already no-op these effects because we wrap ETS calls in `try`
expressions. This change avoids an error log caused by ETS calls
attempting to insert or delete records in an `undefined` table.
@the-mikedavis the-mikedavis force-pushed the md/projections/handle-undefined-tables branch from 2b672cb to cead845 Compare September 10, 2024 15:23
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.68%. Comparing base (e31b236) to head (cead845).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #293      +/-   ##
==========================================
+ Coverage   89.64%   89.68%   +0.03%     
==========================================
  Files          21       21              
  Lines        3187     3188       +1     
==========================================
+ Hits         2857     2859       +2     
+ Misses        330      329       -1     
Flag Coverage Δ
erlang-25 88.92% <100.00%> (+0.03%) ⬆️
erlang-26 89.49% <100.00%> (-0.03%) ⬇️
erlang-27 89.61% <100.00%> (+0.09%) ⬆️
os-ubuntu-latest 89.68% <100.00%> (+0.03%) ⬆️
os-windows-latest 89.55% <100.00%> (+<0.01%) ⬆️

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.

@the-mikedavis the-mikedavis added the bug Something isn't working label Sep 10, 2024
Comment thread src/khepri_projection.erl
@dumbbell dumbbell merged commit c5722bf into main Sep 11, 2024
@dumbbell dumbbell deleted the md/projections/handle-undefined-tables branch September 11, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants