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

[Nikita Pettik]

On 17 Oct 17:12, Konstantin Osipov wrote:
> * Nikita Pettik <korablev at tarantool.org> [19/10/17 17:06]:
> > You made is_force_recovery be global. However, there's a lot of functions
> > taking force_recovery param. Please, refactor them: remove that argument
> > and use instead global status. What is more, you don't set is_force_recovery
> > to false after recovery process is finished. What is more, AFAIR we discussed
> > replacement of memtx_recovery_state with one global state shared among
> > all engines (instead, you added is_recovery_complete).
> 
> Why do you think that now that there is a global state it would be
> good to use it everywhere?

Em, I don't see any pros of duplicating global state by passing it
as a parameter to functions. Meanwile using local version of force_recovery
may turn out to be confusing (at least for developers looking into code).

> Besides, given that now there is is_force_recovery and
> is_recovery_complete, maybe have:
> 
> engine_is_force_recovery() 
> engine_is_recovery_complete() 
> 
> declared in engine.h, rather than directly access global
> variables?

It's OK.