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

[Vladimir Davydov]

On Wed, Jan 31, 2018 at 05:33:10PM +0300, Konstantin Belyavskiy wrote:
> Remove redundancy (second check)
> Decided not to rename in xdir_scan() as it checks only header, but not data rows.

OK

> From 0cf1e8263a28766319895108de2d17cf9d972e23 Mon Sep 17 00:00:00 2001
> From: Konstantin Belyavskiy <k.belyavskiy at tarantool.org>
> Date: Thu, 25 Jan 2018 15:46:22 +0300
> Subject: [PATCH] Fix force_recovery on empty xlog
> 
> * Fix force_recovery behaviour on empty xlog files and ones with corrupted header.
> * Add a test
> 
> Closes #3026, #3076
> ---
>  src/box/recovery.cc               | 25 +++++-----
>  src/box/xlog.c                    |  2 -
>  test/xlog/force_recovery.lua      |  8 ++++
>  test/xlog/force_recovery.result   | 98 +++++++++++++++++++++++++++++++++++++++
>  test/xlog/force_recovery.test.lua | 47 +++++++++++++++++++
>  5 files changed, 167 insertions(+), 13 deletions(-)
>  create mode 100644 test/xlog/force_recovery.lua
>  create mode 100644 test/xlog/force_recovery.result
>  create mode 100644 test/xlog/force_recovery.test.lua
> 
> diff --git a/src/box/recovery.cc b/src/box/recovery.cc
> index 281ac1838..c3a6bb367 100644
> --- a/src/box/recovery.cc
> +++ b/src/box/recovery.cc
> @@ -330,20 +330,24 @@ 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

The comment needs polishing. Please explain why we need to do this and
why it is supposed to work.

>  	 */
> -	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) {

I think you don't need this check any more.

>  		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) {
> +			say_info("rename corrupted xlog %s", name);
> +			char to[PATH_MAX];
> +			snprintf(to, sizeof(to), "%s.corrupted", name);
> +			if (rename(name, to) != 0) {
> +				tnt_raise(SystemError,
> +					  "%s: can't rename corrupted xlog",
> +					  name);
> +			}
>  		}
>  	}
>  }