Re: [Tarantool-patches] [PATCH v2 1/1] txn: destroy commit and rollback triggers

[Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

25.04.2021 18:42, Vladislav Shpilevoy пишет:
> They were not deleted ever. Worked fine for DDL and replication,
> for which they were introduced in the first place, because these
> triggers are on the region memory.
>
> But didn't work when the triggers became available in the public
> API, because these are allocated on the heap. As a result, all the
> box.on_commit() and box.on_rollback() triggers leaked.
>
> The patch ensures all the on_commit/on_rollback triggers are
> destroyed.
>
> The statement triggers on_commit/on_rollback are left intact since
> they are private and never need deletion, but the patch adds
> assertions in case they ever would need to be destroyed.
>
> Another option was to force all the commit and rollback triggers
> clear themselves. For example, in case of commit all the on_commit
> triggers must clear themselves, and the rollback triggers are
> destroyed. Vice versa when a rollback happens. This would allow
> not to manually destroy on_commit triggers in case of commit. But
> it didn't work because the Lua triggers all work via a common
> runner lbox_trigger_run(), which can't destroy its argument in
> most of the cases (space:on_replace, :before_replace, ...). It
> could be patched but requires to add some work to the Lua triggers
> affecting all of them, which in total might be not better.
>
> Closes #6025
> ---
> Changes in v2:
> - Fixed a bug about txn rollback triggers called at statement
>    rollback, added a test;
> - Removed attempts to destroy and clear statement triggers;
>
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-6025-box.on_commit-leak
> Issue: https://github.com/tarantool/tarantool/issues/6025
>

Hi! Thanks for the fixes! LGTM.

-- 
Serge Petrenko