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

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Thu Jun 3 15:55:05 MSK 2021


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.


More information about the Tarantool-patches mailing list