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

[v.shpilevoy at tarantool.org]

> 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. Do you want to copy it in each mem and run and range? It is discussed with Kostja, and he has approved key_def reference counting. Kostja, correct me, please, if I misunderstood you?

> 
> Anyway, this isn't enough. Apart from worker threads, there are also
> reader fibers, which access key_def too. Referencing key_def for them is
> likely to turn the code into even a bigger mess than it is with this
> patch...

I can not find places in a code, where key_def has no 'index->' before it, except workers (write_iterator), mems, ranges and cache. For workers I moved key_def into struct task. For other places I update key_def in commit_alter(). Where key_def is used in Tx fibers instead of different threads via index->key_def, there is no problems.

> 
> IMO we should simply disable this feature for vinyl for now - I don't
> think anybody will give a damn.

Disabling this feature for vinyl is breaking change. Actually, we have forbid to reset space format:

box.cfg{}
s = box.schema.create_space('test', {engine = 'vinyl'})
format = {}
format[1] = {'field1', 'unsigned'}
format[2] = {'field2', 'unsigned', is_nullable = true}
format[3] = {'field3', 'unsigned'}
s:format(format)
pk = s:create_index('pk')
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.
>>