Re: [Tarantool-patches] [PATCH 2/2] fiber: destroy fiber.storage created by iproto

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the review!

Disclaimer: in case something looks 'toxic', this is an
accident, and is not done intentionally.

On 09/12/2019 08:21, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/12/08 23:47]:
> 
> The danger of reporting, self-assigning and fixing a bug is that
> ... it might be not a bug.

I agree. Except when something is obviously a bug which is
clearly the case.

> 
> Users should know that iproto fibers are pooled, not
> created/destroyed on demand. So fiber local storage is simply not
> making any sense for them.

Here I disagree. Users should not know anything about whether
the fibers are pooled, when, and how.

If so, then please, give an example, when such a strange
fiber.storage is needed? I just really can't come up with a
usage case.

I did a public poll to understand what a behaviour is expected,
and not a single person expects, that pooled vs not pooled fiber
would affect its behaviour. Pool is like a cache. It should not
be visible. You literally said it in another thread about SQL
PREPARE.

> 
> A proper fix would be to disable fiber local storage for these
> fibers, not try to make it work.

That would lead to a problem, that functions, using fiber.storage,
would work properly being called from a background fiber, but would
fail being called from an iproto-used fiber. It is clearly not a
fix, but rather a much worse bug, sorry.

> 
> Even though your patch "works", it is pretty much useless.
> 

Well, sorry, with all respect, but I disagree here too. Appeared,
there were repetitive requests to fix this weird behaviour of
fiber.storage a couple of years ago, and again recently, which were
successfully rejected by you and by the same reason, that pooling,
as you think, should be exposed.

My patch fixes that problem, what makes it not useless.

Moreover, that fixes another problem about pooled fibers never
freeing the storages (= leak) and therefore occupying Lua
memory, which is very limited in size. That may become a real
problem, when there are many pooled fibers. Their count may be
thousands, and depends on net_msg_max.