Re: [Tarantool-patches] [PATCH v2 3/3] sql: update temporary file name format

[Sergey Ostanevich <sergos@tarantool.org>]

Thanks for the patch!

As I mentioned in another thread - this one is LGTM.

Sergos

> On 14 Dec 2020, at 18:54, Leonid Vasiliev <lvasiliev@tarantool.org> wrote:
> 
> Hi! Thank you for the review.
> 
> On 13.12.2020 21:30, Vladislav Shpilevoy wrote:
>> Thanks for the patch!
>> On 11.12.2020 15:49, Leonid Vasiliev via Tarantool-patches wrote:
>>> The bug was consisted in fail when working with temporary files
>>> created by VDBE to sort large result of a `SELECT` statement with
>>> `ORDER BY`, `GROUP BY` clauses.
>>> 
>>> Whats happen (step by step):
>>> - We have two instances on one node (sharded cluster).
>>> - A query is created that executes on both.
>>> - The first instance creates the name of the temporary file and
>>>   checks a file with such name on existence.
>>> - The second instance creates the name of the temporary file
>>>   (the same as in  first instance) and checks a file with such name
>>>   on existence.
>>> - The first instance creates a file with the `SQL_OPEN_DELETEONCLOSE`
>>>   flag.
>>> - The second instance opens(try to open) the same file.
>>> - The first instance closes (and removes) the temporary file.
>>> - The second instance tries to work with the file and fails.
>>> 
>>> Why did it happen:
>>> The temporary file name format has a random part, but the random
>>> generator uses a fixed seed.
>>> 
>>> When it was decided to use a fixed seed:
>>> 32cb1ad298b2b55d8536a85bdfb3827c8c8739e1
>> To reference a commit we also usually include commit title in
>> ("...") after the hash.
> 
> Changed to:
> 
> When it was decided to use a fixed seed:
> 32cb1ad298b2b55d8536a85bdfb3827c8c8739e1
> ("sql: drop useless code from os_unix.c")
> 
>> The patch itself looks good.