[tarantool-patches] Re: [PATCH v3 3/4] box: do not evaluate ck constraints on recovery

[Konstantin Osipov]

* Kirill Shcherbatov <kshcherbatov at tarantool.org> [19/09/16 15:49]:
>  	}
> +	is_recovery_completed = true;
>  	fiber_gc();

I think is_recocvery_complete is correct.

https://huybien.com/complete-vs-completed/

> index b422c2f3e..dabd4b1b5 100644
> --- a/src/box/ck_constraint.c
> +++ b/src/box/ck_constraint.c
> @@ -31,6 +31,7 @@
>  #include "memtx_engine.h"
>  #include "box/session.h"
>  #include "bind.h"
> +#include "cfg.h"
>  #include "ck_constraint.h"
>  #include "errcode.h"
>  #include "schema.h"
> @@ -225,7 +226,8 @@ ck_constraint_new(struct ck_constraint_def *ck_constraint_def,
>  	}
>  	ck_constraint->def = NULL;
>  	ck_constraint->stmt = NULL;
> -	ck_constraint->is_enabled = true;
> +	ck_constraint->is_enabled = is_recovery_completed ||
> +				    cfg_geti("force_recovery");

Please pass "enable" as ck_constraint_new argument, and make the
decision at the caller, rather than add a cfg.h dependency.

> +static int
> +space_enable_ck_constraints(struct space *space, void *param)
> +{
> +	(void) param;
> +	struct ck_constraint *ck;
> +	rlist_foreach_entry(ck, &space->ck_constraint, link)
> +		ck->is_enabled = true;
> +	(void) trigger_run(&on_alter_space, space);

Why don't you use space_ck_constaint_set_state here?


-- 
Konstantin Osipov, Moscow, Russia