khepri_tree: Add an option to accumulate keep-while expirations #301
Conversation
This change adds a type `khepri:delete_options()` similar to
`khepri:put_options()` which includes a new option
`return_keep_while_expirations`. Setting this option in a call to
`khepri_adv:delete_many/3` or `khepri_tx_adv:delete_many/2` will include
any tree nodes deleted because of expired keep-while conditions in the
`khepri_adv:many_results()` return value.
In order to accumulate these deletions we need to modify
`khepri_machine:remove_expired_nodes/2` slightly to pass the fold
function and accumulator for the current `#walk{..}`.
This option is useful for callers if they need to do something with any
tree nodes which were deleted by keep-while conditions. For example in
RabbitMQ we currently delete all bindings which are associated with a
queue in a transaction. Bindings should always be removed when their
associated queue is removed so they seem like an ideal case to use a
keep-while condition. We send notifications and invoke callbacks with
the set of bindings which were deleted though, so we need to return
these deleted records. With this change we can avoid performing binding
deletion in a transaction and instead only delete the queue record while
providing the new option. This is significantly more efficient because
of the way the Khepri store is organized in RabbitMQ.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #301 +/- ##
==========================================
+ Coverage 89.60% 89.71% +0.10%
==========================================
Files 22 22
Lines 3251 3256 +5
==========================================
+ Hits 2913 2921 +8
+ Misses 338 335 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Expanding a bit on the efficiency aspect for the use-case in RabbitMQ... When deleting a queue we call [rabbitmq,
vhosts, VHostName,
exchanges, _SrcName = ?KHEPRI_WILDCARD_STAR,
bindings, queue, QName, _RoutingKey = ?KHEPRI_WILDCARD_STAR]So in order to find all bindings that lead to a given queue we need to search each exchange's subtree even if the exchange has no bindings. It is fast to walk down each subtree individually but the cost becomes noticeable when the metadata store has many exchanges and you attempt to delete many queues. For example deleting a vhost that has 100k exchanges is fast, deleting a vhost with 100k queues is pretty fast, but deleting a vhost with both 100k exchanges and 100k queues is very slow. (Note that vhost deletion currently deletes queues and then exchanges.) Using keep-while conditions to clean up these bindings means that we use the prefix tree in #298 instead which is a much more efficient lookup because the prefix tree associates the queue directly to all bindings that need to be deleted. Complexity-wise the time taken to delete a queue is currently proportional to the number of exchanges in the queue's vhost but with this change we can make the time taken proportional only to the number of bindings which point to the queue. |
|
Thank you, I like that we get a way to get all nodes deleted by However, I’m undecided on the use of an option. First, I think I prefer if we have a single behavior because it’s easier for users to reason about and reduce the complexity of the library code. Second, a put could trigger the delete of tree nodes too if a I need to think about this. |
|
Superseded by #305 |
This change adds a type
khepri:delete_options()similar tokhepri:put_options()which includes a new optionreturn_keep_while_expirations. Setting this option in a call tokhepri_adv:delete_many/3orkhepri_tx_adv:delete_many/2will include any tree nodes deleted because of expired keep-while conditions in thekhepri_adv:many_results()return value.In order to accumulate these deletions we need to modify
khepri_machine:remove_expired_nodes/2slightly to pass the fold function and accumulator for the current#walk{..}.This option is useful for callers if they need to do something with any tree nodes which were deleted by keep-while conditions. For example in RabbitMQ we currently delete all bindings which are associated with a queue in a transaction. Bindings should always be removed when their associated queue is removed so they seem like an ideal case to use a keep-while condition. We send notifications and invoke callbacks with the set of bindings which were deleted though, so we need to return these deleted records. With this change we can avoid performing binding deletion in a transaction and instead only delete the queue record while providing the new option. This is significantly more efficient because of the way the Khepri store is organized in RabbitMQ.