> On 9 Jan 2020, at 10:47, Willy Tarreau <[email protected]> wrote:
>
> Hi Thierry,
>
> On Thu, Jan 09, 2020 at 10:34:35AM +0100, Thierry Fournier wrote:
>> I'm remember: the execution of the GC were the only way to close TCP
>> connexions
>> because in some cases the object provided by core.tcp() is not closed and the
>> connexion is not released. Without the GC execution, HAProxy reach the
>> maximum
>> of available connexion, and the process were blocked. The flag forcing the
>> GCC
>> is set only is we use a socket.
>
> Ah OK then it makes sense, thanks for the explanation. However I'm seeing
> that the GC is forced for every yield/resume call, and not just on destroy.
> Maybe it could be acceptable not to call it on resume ?
>
> Sada, would you be interested in checking if this solves most of the
> performance issue:
>
> diff --git a/src/hlua.c b/src/hlua.c
> index 37f7866874..e5257efb54 100644
> --- a/src/hlua.c
> +++ b/src/hlua.c
> @@ -1195,7 +1195,7 @@ resume_execution:
> }
>
> /* This GC permits to destroy some object when a Lua timeout strikes.
> */
> - if (lua->flags & HLUA_MUST_GC &&
> + if (0 && lua->flags & HLUA_MUST_GC &&
> ret ! HLUA_E_AGAIN)
> lua_gc(lua->T, LUA_GCCOLLECT, 0);
This make sense, but I not remember if the GC at this point is important.
Just for information, there are the commit messages:
OPTIM/MEDIUM: lua: executes the garbage collector only when using cosocket
The garbage collector is a little bit heavy to run, and it was added
only for cosockets. This patch prevent useless executions when no
cosockets are used.
>
>> Maybe we try to include new option "tune.lua.forced-gc".
>
> Based on your description if we need an option, I'd rather have the
> opposite, like "tune.lua.disable-gc" so that it remains enabled by
> default, and with an explanation that one must not disable it.
I agree, but it make sense to run some tests:
- First, testing your patch to decide if the GC line 1195 is relevant
- Second, testing the behavior when GC is disabled and lua create connexion
and lua fail without closing connexion.
Maybe the second case make the GC mandatory.
Thierry
