From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id B04034696C3
 for <tarantool-patches@dev.tarantool.org>;
 Thu,  2 Apr 2020 03:29:48 +0300 (MSK)
References: <cover.1585097339.git.korablev@tarantool.org>
 <d0a6aaffaaaea96d81a0cd006b9aeb762b2bcea7.1585097339.git.korablev@tarantool.org>
 <8620360c-c99d-f9ea-e7f4-25c97c00c0f1@tarantool.org>
 <20200331174407.GA22927@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <0f8cecd0-e483-8ee4-be3b-c848ec9009ae@tarantool.org>
Date: Thu, 2 Apr 2020 02:29:46 +0200
MIME-Version: 1.0
In-Reply-To: <20200331174407.GA22927@tarantool.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH v2 06/10] box: introduce stacked
 diagnostic area
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Nikita Pettik <korablev@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org

Thanks for the answers!

On 31/03/2020 19:44, Nikita Pettik wrote:
> On 28 Mar 19:59, Vladislav Shpilevoy wrote:
>> Two more comments.
>>
>>> diff --git a/test/box/error.test.lua b/test/box/error.test.lua
>>> index a0b7d3e78..1fdd6ed98 100644
>>> --- a/test/box/error.test.lua
>>> +++ b/test/box/error.test.lua
>>> @@ -108,4 +108,109 @@ box.error.new(err)
>>> +
>>>  space:drop()
>>
>> 1. You probably need to keep this 'space:drop()' after
>> the test related to it.
> 
> Isn't it too late?:) I mean test it is related to is finished at
> line 20, meanwhile space:drop() is already at line 111 (box/error.test.lua
> is already pushed).

You still can add 1148 tests after space:drop, not before. In
that case it at least won't become worse than it is.

> Apllied your diff with one change and every launch I get:
> 
> [001] +errcount
> [001] + | ---
> [001] + | - 1
> [001] + | ...
> [001] +errcount2
> [001] + | ---
> [001] + | - 1
> [001] + | ...
> [001] +
> 
> One error I guess is that which gets stuck in diag. The change is:
> 
> @@ -124,6 +129,7 @@ error_unref(struct error *e)
>                 to_delete->cause = NULL;
>                 to_delete->effect = NULL;
>                 to_delete->destroy(to_delete);
> +               __atomic_add_fetch(&diag_error_count, -1, __ATOMIC_SEQ_CST);
>                 if (cause == NULL)
>                         return;
>                 to_delete = cause;
>  
> I.e. we should accout destroy before checking cause since for
> the last error in the list it is NULL, ergo it won't be accounted.

Shame on me, you are right. I should decrement it exactly on
destroy. Then there is no leaks!

> Nit: could you please send diff as a separate mail attachment next time?
> The thing is I have to manually extract it to a separate file
> instead of being capable of applying it as a patch via git am/apply.
> Thanks.

Sure.