maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Aleksey!

On Jan 13, Aleksey Midenkov wrote:
> >
> > But always using expr_arena doesn't necessarily seem to be a correct
> > choice. E.g. Arg_comparator creates Item_cache_temporal on every
> > fix_field. If vcol expr is re-fixed all the time, expr_arena will
> > grow continuously. Same for charset conversion, CASE, IN, etc. I
> > suspect that if the vcol expr is always re-fixed, it has to use stmt
> > arena.
> 
> stmt_arena cannot be used as long as 'expr' item is created on
> expr_arena. 'expr' item is created at the vcol expression parse and is
> reused during the whole TABLE lifetime. If some of the containee items
> later were allocated on stmt_arena, 'expr' item will try to access
> them in a different statement when they are already released. That is
> what this bugfix tries to address.
> 
> Yes, this fix introduces a small and in most cases imperceptible
> memory leak which can be easily overcame by FLUSH TABLES. The fix for
> this leak is non-trivial.
> 
> 1. vcol expr is not always refixed, but conditionally controlled by
> vcols_need_refixing. And we must remove this control and truly refix
> always.
> 
> 2. we must clone 'expr' item into statement mem_root at each
> statement.
> 
> I have attached the leak fix PoC into this email. I do not know
> whether this is worth it.

vcols_need_refixing isn't run-time conditional, it depends on vcol
flags, a specific vcol always needs refixing or never does.
And a table has vcols_need_refixing if any of the vcols nees it.

Thus, I think, a simpler fix would be to split the cleanup and
fix_fields. Now both are done in fix_session_vcol_expr().

We can do cleanup at the end of every statement and fix_fields at the
beginning of a statement. Just like for normal non-persistent items.
For every vcol that needs it. This way they can safely use execution
arena.

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx