Re[3]: [patches] Re: [PATCH] Fix force_recovery on empty xlog

[Konstantin Belyavskiy]

Remove redundancy (second check)
Decided not to rename in xdir_scan() as it checks only header, but not data rows.


>Среда, 31 января 2018, 17:05 +03:00 от Konstantin Belyavskiy <k.belyavskiy at tarantool.org>:
>
>
>>
>>> 
>>>  /*
>>>   * Recovery subsystem
>>> @@ -330,20 +332,26 @@ recovery_finalize(struct recovery *r, struct xstream *stream)
>>>  	recovery_close_log(r);
>>> 
>>>  	/*
>>> -	 * Check that the last xlog file has rows.
>>> +	 * Rename last corrupted xlog if any. Cases:
>>> +	 *  - file has corrupted rows
>>> +	 *  - file has corrupted header
>>> +	 *  - file has zero size
>>>  	 */
>>> -	if (vclockset_last(&r->wal_dir.index) != NULL &&
>>> -	    vclock_sum(&r->vclock) ==
>>> -	    vclock_sum(vclockset_last(&r->wal_dir.index))) {
>>> -		/*
>>> -		 * Delete the last empty xlog file.
>>> -		 */
>>> +	if (vclockset_last(&r->wal_dir.index) != NULL) {
>>>  		char *name = xdir_format_filename(&r->wal_dir,
>>>  						  vclock_sum(&r->vclock),
>>>  						  NONE);
>>> -		if (unlink(name) != 0) {
>>> -			tnt_raise(SystemError, "%s: failed to unlink file",
>>> -				  name);
>>> +		if (access(name, F_OK) == 0 ||
>>
>>> +		    vclock_sum(&r->vclock) ==
>>> +		    vclock_sum(vclockset_last(&r->wal_dir.index))) {
>>
>>If there's an xlog file corresponding to r->vclock in r->wal_dir.index,
>>it must exist, i.e. access() must return 0 for it. That said, the second
>>check looks redundant.
>>
>>Come to think of it, we have scanned the whole xlog directory in
>>xdir_scan() by the time we get here so using access() here looks
>>strange. Can we call rename() right from xdir_scan() for files that
>>we failed to index (corrupted header, empty)?
>>
>If xlog has corrupted data but header is OK, when second check
>vclock_sum(&r->vclock) == vclock_sum(vclockset_last(&r->wal_dir.index))
>
>But should we rename on xdir_scan() ? Good question BTW..
>Roman, Konstantin, what do you think?
>
>
>С уважением,
>Konstantin Belyavskiy
>k.belyavskiy at tarantool.org


С уважением,
Konstantin Belyavskiy
k.belyavskiy at tarantool.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20180131/9d218f3c/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-force_recovery-on-empty-xlog.patch
Type: application/x-patch
Size: 6063 bytes
Desc: not available
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20180131/9d218f3c/attachment.bin>