We're going to need to test it anyways whether it's at the call or definition site and modifying the eqc test for this should be pretty straight forward. Plus the eqc test already exists. It seems to me that the new surface area is either spread over the call sites or the definition site (bg mgr) since the latter is new code IMO vs changing established code even more it makes more sense to localize the changes to the bg mgr and reduce repetition at least to me.

All in all the, I'm somewhat indifferent if it continues to work. 

Sent from my iPhone

> On Dec 18, 2013, at 12:42 PM, Chris T <notifications@github.com> wrote:
> 
> In src/riak_kv_index_hashtree.erl:
> 
> > +%%      settings are defined as anything other than false. 'skip_background_manager'
> > +%%      turns off all use of bg-mgr. 'aae_skip_background_manager' only stops
> > +%%      AAE tree rebuilds from using bg-mgr.
> > +-spec use_bg_mgr() -> boolean().
> > +use_bg_mgr() ->
> > +    %% note we're tolerant here of any non-boolean value as well. Iff it's 'false', we don't skip.
> > +    GlobalSkip = app_helper:get_env(riak_core, skip_background_manager, false) =/= false,
> > +    AAESkip = app_helper:get_env(riak_core, aae_skip_background_manager, false) =/= false,
> > +    not (GlobalSkip orelse AAESkip).
> > +
> > +%% @private
> > +%% @doc Unless skipping the background manager, try to acquire the per-vnode lock.
> > +%%      Sets our task meta-data in the lock as 'aae_rebuild', which is useful for
> > +%%      seeing what's holding the lock via @link riak_core_background_mgr:ps/0.
> > +-spec maybe_get_vnode_lock(SrcPartition::integer()) -> ok | max_concurrency.
> > +maybe_get_vnode_lock(SrcPartition) ->
> While I don't mind moving the global check into get_lock, I don't like moving this into the arguments of get_lock, unless we're not planning on writing more eqc tests for it. Having a function in bg-mgr that does the checks for you is cool, though. Otherwise, it's going to get messy. We'd need to add these to more of the API, at least get_token, and set_concurrency_limit. More surface area.
> 
> I propose that I create two additional helper functions in bg-mgr that do the config checking, but the call points still do the tests.
> 
> —
> Reply to this email directly or view it on GitHub.