[Tarantool-patches] [PATCH 2/2] vinyl: skip invalid upserts during squash

[Vladislav Shpilevoy]

Thanks for the fixes!

On 14/05/2020 04:21, Nikita Pettik wrote:
> On 01 May 02:31, Vladislav Shpilevoy wrote:
>> Hi! Thanks for the patch!
>>
>> Firstly, Kostja left some comments here. Would be cool to address them.
> 
> Done (sorry, I did not ignore them, just had to work on other more vital bugs).
>  
>> Secondly, here is my personal opinion. I don't like just skipping things
>> a user committed without any error appearing in the application. IMO, we
>> should apply only the first commit. And let a user see this error so as he
>> could notice the problem. To fix reads he could do delete() of the bad key.
> 
> The problem with delete it leaves user no way to restore the rest
> of upsert history. Moreover, these upserts will get stuck until
> user finds in logs corresponding error (I guess we can't abort
> compaction due to invalid upserts).
> 
>> However, how a user will be able to find the exact broken key - I don't
>> know. Maybe the ignore + logging is better.
> 
> Why can't we just log broken key? E.g. see logs in vy_apply_upsert().

We can log it. This is what you do in this patchset.

I also noticed, that you skip the failed upsert in case of any error.
Even if it is an OOM, not related to format problems. I think it is safer
to check error type, and skip it only in case of a ClientError.