-
Notifications
You must be signed in to change notification settings - Fork 233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Integrate Background Manager with Handoff And AAE Tree Rebuilds #769
Conversation
%% 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() -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is duplicated in both riak_kv
and riak_repl
. Can't this be in core, and take a parameter instead of what property to inspect in the environment, if required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for the global switch, you have a good point. I'll add this entry to the background manager.
try_set_concurrency_limit(Lock, Limit, riak_core_bg_manager:use_bg_mgr()). | ||
|
||
try_set_concurrency_limit(_Lock, _Limit, false) -> | ||
lager:info("Skipping background manager."), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is probably going to get verbose as an info
statement since its per-partition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh heck. I meant to make that a debug. I wonder if we even need it all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think we need it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed by a78aee5
+1 merge after fixing the I tested this mostly by hand because unfortunately there aren't many riak_test's that cover this. I verified that AAE rebuilds and handoff's don't stomp on each other via redbug ( Final Comments:
|
Integrate Background Manager with Handoff And AAE Tree Rebuilds
NOTE: DO NOT MERGE YET (this PR requires code from #475 or basho/riak_core#484)
The riak_core handoff portion is here: basho/riak_core#484 (includes #475)
The riak_core background manager PR is here: basho/riak_core#475
The riak_repl PR is here: basho/riak_repl#488
The background manager allows subsystems to coordinate with locks (and tokens) in a non-blocking API when desired. In our case, we've seen several customer support issues related to a production cluster being overloaded by the combined weight of MDC fullsync replication in combination with either handoff, or AAE tree rebuilds (or all three!).
Each of these subsystems is being taught, for 2.0, to "take" a lock on the kv vnode/partition before it does a fold over that partition. Locks are returned when the process dies or exists normally. Each subsystem (at this point in time) is coded to keep working on other partitions if one partition is denied a lock via the return value of max_concurrency, which means progress is made even when one partition is busy.
Use of this feature can be bypassed in production. Besides a global skill switch on the background manager itself, the riak_core configuration supports two new parameters: aae_skip_bg_manager and handoff_skip_bg_manager, which defaults to false. Setting it to true will not call the background manager and proceed like it used to.
I chose not to separate the AAE and Handoff into two separate PRs because we already have too many, but if we determine that we want to exclude the handoff portion, I'll spit them up.