[patches] [PATCH 4/4] vinyl: on space alter update key definitions in indexes

v.shpilevoy at tarantool.org v.shpilevoy at tarantool.org
Mon Mar 12 20:01:21 MSK 2018



> 12 марта 2018 г., в 19:47, Vladimir Davydov <vdavydov.dev at gmail.com> написал(а):
> 
> On Mon, Mar 12, 2018 at 07:16:31PM +0300, v.shpilevoy at tarantool.org wrote:
>> 
>> 
>>> 12 марта 2018 г., в 19:10, Vladimir Davydov <vdavydov.dev at gmail.com> написал(а):
>>> 
>>> On Mon, Mar 12, 2018 at 05:41:12PM +0300, v.shpilevoy at tarantool.org wrote:
>>>> 
>>>> 
>>>>> 12 марта 2018 г., в 16:03, Vladimir Davydov <vdavydov.dev at gmail.com> написал(а):
>>>>> 
>>>>> On Mon, Mar 12, 2018 at 12:53:36AM +0300, Vladislav Shpilevoy wrote:
>>>>>> Vinyl alter can change comparators, for example, by removal of a
>>>>>> space format and making some fields optional. This alter is
>>>>>> allowed on a non-empty space even, because optionality of a
>>>>>> field is not persisted. But index key definitions can not be
>>>>>> updated in place, since they are used in multiple threads (vinyl
>>>>>> workers). So it is necessary to get new key definitions from a
>>>>>> new index definition, and leave old ones to be deleted by
>>>>>> existing worker tasks, when they will be finished.
>>>>> 
>>>>> I don't think that workers alone are worth introducing key_def reference
>>>>> counting - we can just copy key_def for them, it is a slow path after
>>>>> all.
>>>> 
>>>> We can, but further we want to store key_def per mem and per run to
>>>> allow various formats and keys in different places.
>>> 
>>> But why? All tuples stored in an index should have the same format and
>>> be compared by the same key definition, no?
>> 
>> Now yes. Later - no. On vinyl alter you can not update formats in all older tuples in all mems and runs.
> 
> I don't think I understand. If you change space format, all tuples must
> conform to it, no? Why do we need a separate format per each run and/or
> mem?

Exactly. But they may not be the same. They different, but compatible. I think, that the same can be applied to key_defs in a future. In a case of vinyl we must avoid full rebuild - it will be much and mush slower that memtx, and if key_def will be changed in a compatible way, then I do not see a reason to do not use it for rebuild avoidance.

> 
>> And on index update you can not change key_defs too.
> 
> Why would you need to do anything like this? Just rebuild the whole
> index on key_def alter, that's all. Simple and robust.

It is not effective when a key_def is changed in a compatible way (for example, by extending of part types).

> 
> To be honest, I find all those little tricks, like changing key field
> type from unsigned to signed without rebuild, useless. Anybody needs
> that?

You think, that it useless, but lets see how many time rebuild consumes on a space with, for instance, 5 000 000 000 tuples?

> 
>> 
>>> 
>>>> sk = s:create_index('sk', {parts = {{2, 'unsigned', is_nullable = true}}})
>>>> s:format({}) -- this will fail, because min_field_count is changed. But user even does not know about 'min_field_count'. We simply can not explain to a user, why it is forbidden.
>>> 
>>> Who cares? De facto, vinyl doesn't support alter at all. The user can
>>> drop all indexes and recreate the space with another format.
>> 
>> Vinyl supports alter. At least, it allows to update format in compatible ways.
> 
> You've recently forbidden altering key_def of an empty vinyl index, and
> nobody cared. How's space format any different?

I have forbidden key definition alter, yes, but I did not forbid to create/drop an index. And these hacks are only for empty spaces. On a non-empty space key_definition has no worked ever.
Now you propose to forbid alteration of a non-empty space. And for a reason, that can not be even explained to a user. With the same success we can forbid any alter of Vinyl spaces, except disk options (page sizes etc).

> 
>> 
>>> 
>>> Anyway, the fact that you can't change a key definition or space format
>>> without potentially affecting other key definitions looks cheesy...
>> 
>> A user does not see changes of non-altered indexes. So it is not cheesy.
> 
> I didn't mean users, I meant the ALTER code - it's getting more and more
> convoluted. And the bug you're fixing is a living proof to that.

It always was slightly "convoluted". Somewhere it complicates, and somewhere it simplifies - the ModifySpaceFormat is "living proof to that" - it removed many of doubtful code from ModifySpace.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20180312/e89ef53a/attachment.html>


More information about the Tarantool-patches mailing list