> On 10 Jan 2020, at 04:46, Willy Tarreau <w...@1wt.eu> wrote:
> 
> On Thu, Jan 09, 2020 at 05:30:32PM -0800, Sadasiva Gujjarlapudi wrote:
>> After applying Willy's patch
>> 
>> requests/sec 7181.9869
>> $time haproxy
>>  real 0m10.304s
>>  user 0m1.112s
>>  sys 0m1.184s
>> 
>> After applying Willy's patch and the following patch
>> 
>> @@ -955,10 +956,14 @@ void hlua_ctx_destroy(struct hlua *lua)
>>         * the garbage collection.
>>         */
>>        if (lua->flags & HLUA_MUST_GC) {
>> -               if (!SET_SAFE_LJMP(gL.T))
>> -                       return;
>> -               lua_gc(gL.T, LUA_GCCOLLECT, 0);
>> -               RESET_SAFE_LJMP(gL.T);
>> +               if (lua->gc_tune_count++ > 1000) {
>> +                       lua->gc_tune_count = 0;
>> +                   if (!SET_SAFE_LJMP(gL.T))
>> +                           return;
>> +                       lua_gc(gL.T, LUA_GCCOLLECT, 0);
>> +                   RESET_SAFE_LJMP(gL.T);
>> +               }
>> +
>>        }
>> 
>> requests/sec 8635.0402
>>  real 0m10.969s
>>  user 0m0.744s
>>  sys 0m1.116s
> 
> OK this is promising, thanks for the tests!
> 
>> Most of the time the CPU is the bottleneck and it may be ok to consume
>> little bit more memory on dedicated servers running haproxy.
>> Our use case/Lua application required to use as little CPU as possible.
> 
> Of course, but according to Thierry the problem is not so much about
> memory, but rather about connections/file descriptors.
> 
> Well, I'm thinking about something. Since this HLUA_MUST_GC flag is used
> only for connections, we could probably proceed differently. Instead of
> a flag we could have a connection counter, which gets incremented for
> each connection we create and decremented for each closed connection.
> Then we'd just run the GC in hlua_destroy_ctx() if the connection count
> indicates we did not kill all of them. This way, the GC will never run
> for normal situations where connections were cleanly closed, and it will
> only be run if absolutely needed.
> 
> I can propose you to give a try to the attached patch. I have no idea if
> it works or does something correct. It's only build-tested.


I like this way. If the Lua program works perfectly, the GC is not forced.
If some connections are pending the GC start and clean up.

Maybe it is must clever to rename the variable from “conn_count” to
“require_gc_count”. By this way if another resource have the same problem
than TCP connections, this var could be used ?

For testing this patch we need three cases:

- The Lua program work perfectly.

- The Lua program must fail after connexion (something like this:
  unexistant_array[‘void_key'] = 1). This test the unexpected abort. If the
  Lua fails, we must validate the socket is closed and HAProxy is not blocked
  because no connection available.

- The socket close must be commented out. The same kind of test than previsouly,
  but without the bug.

Thierry

Reply via email to