[Tarantool-patches] [tarantool-patches] Re: [PATCH v4 3/4] box: do not evaluate ck constraints on recovery
Nikita Pettik
korablev at tarantool.org
Thu Oct 17 17:39:30 MSK 2019
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.
More information about the Tarantool-patches
mailing list