Re: [Tarantool-patches] [PATCH vshard 09/11] ref: introduce vshard.storage.ref module

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thanks for the review!

On 24.02.2021 11:28, Oleg Babin wrote:
> Thanks for you patch. It's a brief review - I hope I'll look once again on this patch.
> 
> Consider a question below.

A huge request - could you please remove the irrelevant parts of
the original emails from your responses? I need to scroll tons of
text to find your commets, and can accidentally miss some.

Or at least provide some markers I could grep and jump to quickly.

>> diff --git a/vshard/storage/CMakeLists.txt b/vshard/storage/CMakeLists.txt
>> index 3f4ed43..7c1e97d 100644
>> --- a/vshard/storage/CMakeLists.txt
>> +++ b/vshard/storage/CMakeLists.txt
>> @@ -1,2 +1,2 @@
>> -install(FILES init.lua reload_evolution.lua
>> +install(FILES init.lua reload_evolution.lua ref.lua
>>           DESTINATION ${TARANTOOL_INSTALL_LUADIR}/vshard/storage)
>> diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
>> index c3ed236..2957f48 100644
>> --- a/vshard/storage/init.lua
>> +++ b/vshard/storage/init.lua
>> @@ -1140,6 +1142,9 @@ local function bucket_recv_xc(bucket_id, from, data, opts)
>>               return nil, lerror.vshard(lerror.code.WRONG_BUCKET, bucket_id, msg,
>>                                         from)
>>           end
>> +        if lref.count > 0 then
>> +            return nil, lerror.vshard(lerror.code.STORAGE_IS_REFERENCED)
>> +        end
> 
> 
> You will remove this part in the next patch. Do you really need it? Or you add it just for tests?

For the tests and for making the patch atomic. So as it wouldn't depend on the next
patch.

>>           if is_this_replicaset_locked() then
>>               return nil, lerror.vshard(lerror.code.REPLICASET_IS_LOCKED)
>>           end
>> @@ -1441,6 +1446,9 @@ local function bucket_send_xc(bucket_id, destination, opts, exception_guard)
>>         local _bucket = box.space._bucket
>>       local bucket = _bucket:get({bucket_id})
>> +    if lref.count > 0 then
>> +        return nil, lerror.vshard(lerror.code.STORAGE_IS_REFERENCED)
>> +    end
> 
> 
> Ditto.