Re: [Tarantool-patches] [PATCH v2 06/10] box: introduce stacked diagnostic area

[Vladislav Shpilevoy <v.shpilevoy@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.