Re: [Tarantool-patches] [PATCH 08/17] recovery: recovery_follow_local -- don't throw exception

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch!

See 2 comments below.

> diff --git a/src/box/box.cc b/src/box/box.cc
> index ff095d767..7551ca753 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -2292,8 +2292,9 @@ local_recovery(const struct tt_uuid *instance_uuid,
>  	if (wal_dir_lock < 0) {
>  		title("hot_standby");
>  		say_info("Entering hot standby mode");
> -		recovery_follow_local(recovery, &wal_stream.base, "hot_standby",
> -				      cfg_getd("wal_dir_rescan_delay"));
> +		if (recovery_follow_local(recovery, &wal_stream.base, "hot_standby",
> +					  cfg_getd("wal_dir_rescan_delay")) != 0)

1. This becomes out of 80 symbols now. Lets better wrap some arguments.

> +			diag_raise();
>  		while (true) {
>  			if (path_lock(cfg_gets("wal_dir"), &wal_dir_lock)) {
>  				recovery_stop_local(recovery);
> diff --git a/src/box/recovery.cc b/src/box/recovery.cc
> index 996966a77..24036b7c1 100644
> --- a/src/box/recovery.cc
> +++ b/src/box/recovery.cc
> @@ -568,9 +568,12 @@ recovery_follow_local(struct recovery *r, struct xstream *stream,
>  	 * xlog.
>  	 */
>  	assert(r->watcher == NULL);
> -	r->watcher = fiber_new_xc(name, hot_standby_f);
> +	r->watcher = fiber_new(name, hot_standby_f);
> +	if (!r->watcher)

2. Lets better follow our code style and use explicit '== NULL'. It looks
more understandable, is consistent (it is compared with NULL literally 2
lines above, and in other places in the other patches of your patchset), and
gives a hint on what type the variable in condition has.

> +		return -1;
>  	fiber_set_joinable(r->watcher, true);
>  	fiber_start(r->watcher, r, stream, wal_dir_rescan_delay);
> +	return 0;
>  }