Re: [Tarantool-patches] [PATCH vshard 1/1] recovery: relax recovery messages verbosity

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

Hi! Thanks for the review!

>> diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
>> index 7045d91..8a019fa 100644
>> --- a/vshard/storage/init.lua
>> +++ b/vshard/storage/init.lua
>> @@ -736,21 +736,25 @@ local function recovery_step_by_type(type)
>>       local is_empty = true
> 
> To be honest I don't completely understand what "is_empty" means.

It means the recovery step is empty - didn't find anything to
recover.

I use it so as not to log 'Starting ... buckets recovery step' on
each call of recovery_step_by_type() - would be too many useless
logs.

>>       local recovered = 0
>>       local total = 0
>> +    local start_format = 'Starting %s buckets recovery step'
>>       for _, bucket in _bucket.index.status:pairs(type) do
>>           total = total + 1
>>           local bucket_id = bucket.id
>>           if M.rebalancer_transfering_buckets[bucket_id] then
> 
> Side-note: transfering -> transferring

Too late. There are tons of typos really, in all the code and
comments. Worth making a patch to fix them all, but I am too
lazy for that. Prefer to fix the typos along functional
changes.

>>               goto continue
>>           end
>> -        if is_empty then
>> -            log.info('Starting %s buckets recovery step', type)
>> -        end
>> -        is_empty = false
>>           assert(bucket_is_transfer_in_progress(bucket))
>> -        local destination = M.replicasets[bucket.destination]
>> +        local peer_uuid = bucket.destination
>> +        local destination = M.replicasets[peer_uuid]
>>           if not destination or not destination.master then
>>               -- No replicaset master for a bucket. Wait until it
>>               -- appears.
> 
> This comment states that there is no critical and it's appropriate and we should just wait. Why "error" and not "warn"?

Indeed, sounds fair. Changed to log.warn.