[tarantool-patches] Re: [PATCH 1/1] box: update listen and replication in a strict order

[Vladislav Shpilevoy]

On 29/09/2019 20:35, Georgy Kirichenko wrote:
> On Sunday, September 29, 2019 4:27:37 PM MSK Vladislav Shpilevoy wrote:
>> Hi! Thanks for the review!
> Hi!
> 
> I think Sergey did not say about the order that user used for box.cfg but 
> about our internal options processing order. In the other words, should we 
> define an order in which order box.cfg applies key-values from corresponding 
> table, for instance: first we apply uuid, then checkpoint dir, wal_dir, listen, 
> then replication and so on.

But this is exactly what I have done. I defined an order of box.cfg
keys.

>>
>> On 29/09/2019 09:37, Sergey Ostanevich wrote:
>>> Hi!
>>>
>>> It resolves only these 2 keys order, AFAIU. How about introduce an ordered
>>> table in Tarantool, so that whole cfg and later introduced tables can
>>> preserve the order? An example of
>>> implementation http://lua-users.org/wiki/OrderedTable
>> The problem is that you can't force users write keys in a special
>> order. And what is more, you can't force existing users to change
>> their existing code for that. Additionally, box.cfg may be
>> autogenerated by a script, like vshard does. Making box.cfg
>> order special will require crutches in the scripts to check key
>> names, and insert them in a special order. And finally, box.cfg
>> often is not written as
>>
>>     local cfg = {}
>>     cfg.key1 = value1
>>     cfg.key2 = value2
>>     ...
>>     box.cfg(cfg)
>>
>> Usually it is
>>
>>     box.cfg({
>>         key1 = value1,
>>         key2 = value2,
>>         ...
>>     })
>>
>> So the ordered tables, provided by the link, won't work here.
>> Anyway it would require sorting.
>>
>> So I don't see any sense to implement an ordered table. At least
>> for this case, for box.cfg. Because it just won't help.
>>
>> Yes, my patch fixes only 2 keys, because the problem was with them
>> only. In case we will have more options, order of which should be
>> special, we will add them to this table.
> 
> 
> 
>