Skip to content
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

Add riak_core_ring_manager:is_stable_ring/0 public API [JIRA: RIAK-1598] #716

Merged

Conversation

darach
Copy link
Contributor

@darach darach commented Mar 4, 2015

The status of a ring as stable (no topology change in a promotion period) or not is of general utility.
This patch proposes to refactor the predicate check into a public API so that it can be made more
generally useful.

…nction.

The riak_core_ring_manager already has a mechanism to optimize ring access
during heavy churn to optimize for ETS based access and during heavy reads
to avail of the erlang constant pool via mochiglobal.

The ability to predicate activity on ring stability is however more
generally useful.  For example, certain application commands intended for
cluster wide strongly consistent effect may be ill-advised during periods
of ring topology churn.

This patch simply refactors out the predicate into a public API so that it
can be used as such. The proposed API is:

    riak_core_ring_manager:is_stable_ring() -> boolean().

As the logic is predicated on inactivity and not covered in unit tests no
tests were added as the default inactivity period is hardwired to 90 seconds
and not tunable.
@@ -406,7 +410,10 @@ handle_call({ring_trans, Fun, Args}, _From, State=#state{raw_ring=Ring}) ->
handle_call({set_cluster_name, Name}, _From, State=#state{raw_ring=Ring}) ->
NewRing = riak_core_ring:set_cluster_name(Ring, Name),
State2 = prune_write_notify_ring(NewRing, State),
{reply, ok, State2}.
{reply, ok, State2};
handle_call(is_stable_ring, _From, State) ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Please use spaces after commas in tuples here and below for consistency with surrounding code in this file.

@andrewjstone
Copy link
Contributor

Hi @darach,
Thanks for the PR. If you make the minor changes requested we'll go ahead and get it merged in.

@Basho-JIRA Basho-JIRA changed the title Add riak_core_ring_manager:is_stable_ring/0 public API Add riak_core_ring_manager:is_stable_ring/0 public API [JIRA: RIAK-1598] Mar 10, 2015
@darach
Copy link
Contributor Author

darach commented Mar 10, 2015

Thank you for calling out the 👾's I didn't notice breaking with the surround style.
Your version of the test is better and clearly documents the dependency on ?PROMOTE_TIMEOUT, so adopted. Let me know if there's anything else I can change to more closely reflect the prevailing style and conventions.

@andrewjstone
Copy link
Contributor

👍 3666df2

Thanks for making those changes @darach

borshop added a commit that referenced this pull request Mar 10, 2015
…g_manager

Add riak_core_ring_manager:is_stable_ring/0 public API [JIRA: RIAK-1598]

Reviewed-by: andrewjstone
@andrewjstone
Copy link
Contributor

@borshop merge

@borshop borshop merged commit 3666df2 into basho:develop Mar 10, 2015
@darach
Copy link
Contributor Author

darach commented Mar 10, 2015

Awesome! 💕

@mbbroberg
Copy link

^ Love seeing these moments 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants