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.

Sign up for GitHub

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

Fixes/error no members handling #36

Merged
merged 5 commits into from
Apr 7, 2016
Merged

Fixes/error no members handling #36

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
14618ea
Squashed commit of the following:
Mar 31, 2016
ca5a087
Merge branch 'refs/heads/master' into fixes/error_no_members_handling
Apr 4, 2016
de29a8d
prevent from having warning on function with default value for params
Apr 4, 2016
128c611
update group_member retrieval following PR review
Apr 7, 2016
9d8cdc3
removed seed in connection init
Apr 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/riak/connection.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,21 @@ defmodule Riak.Connection do
Create a linked process to talk with the riak server on host:port.
"""
def start_link(host \\ '127.0.0.1', port \\ 8087, args \\ []) do
init_seed()
:riakc_pb_socket.start_link(host, port, args)
end

@doc """
Create a process to talk with the riak server on host:port.
"""
def start(host \\ '127.0.0.1', port \\ 8087, args \\ []) do
init_seed()
:riakc_pb_socket.start(host, port, args)
end

defp init_seed do
# Let's initialize a seed for pseudo uniform random number - no periodic
<< a :: 32, b :: 32, c :: 32 >> = :crypto.rand_bytes(12)
Copy link
Owner

Choose a reason for hiding this comment

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

With the timeout mechanism in place, it doesn't seem like we need to mess with the random number seed anymore right? Or were you leaving this here so that the downstream pooler stuff benefits it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, you are right! I'll remove it.

Le 7 avr. 2016 à 18:30, Drew Kerrigan [email protected] a écrit :

In lib/riak/connection.ex:

 :riakc_pb_socket.start(host, port, args)

end
+

  • defp init_seed do

  • Let's initialize a seed for pseudo uniform random number - no periodic

  • << a :: 32, b :: 32, c :: 32 >> = :crypto.rand_bytes(12)
    With the timeout mechanism in place, it doesn't seem like we need to mess with the random number seed anymore right? Or were you leaving this here so that the downstream pooler stuff benefits it?

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

:random.seed(a, b, c)
end
end
6 changes: 4 additions & 2 deletions lib/riak/crdt/counter.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,17 @@ defmodule Riak.CRDT.Counter do
@doc """
Increment a `counter` on the `amount` defaulting in 1
"""
def increment(counter, amount \\ 1) when Record.is_record(counter, :counter) do
def increment(counter, amount \\ 1)
Copy link
Owner

Choose a reason for hiding this comment

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

What is the purpose of this additional increment def, and the decrement below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It prevents from having a warning at compile time on that function because it has default value for the 'amount' param and several clauses.
I believe this warning is issued since elixir 1.2

Copy link
Owner

Choose a reason for hiding this comment

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

Makes sense, 👍

def increment(counter, amount) when Record.is_record(counter, :counter) do
:riakc_counter.increment(amount, counter)
end
def increment(nil, _), do: {:error, :nil_object}

@doc """
Decrement a `counter` on the `amount` defaulting in 1
"""
def decrement(counter, amount \\ 1) when Record.is_record(counter, :counter) do
def decrement(counter, amount \\ 1)
def decrement(counter, amount) when Record.is_record(counter, :counter) do
:riakc_counter.increment(-amount, counter)
end
def decrement(nil, _), do: {:error, :nil_object}
Expand Down
8 changes: 7 additions & 1 deletion lib/riak/pool.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ defmodule Riak.Pool do
end
end
def unquote(name)(unquote_splicing(other_args)) do
pid = :pooler.take_group_member(:riak)
pid = case :pooler.take_group_member(:riak) do
:error_no_members ->
:timer.sleep(100 + :random.uniform(50)) # waits for a random time from 100ms to 150ms
Copy link
Owner

Choose a reason for hiding this comment

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

Sleeping a random amount of time is almost never the right answer, except when it is :-). Could you elaborate on how this changes the effect of the subsequent call to take_group_member? Also could you explain why we're confident that the subsequent call to take_group_member will not also return :error_no_members?

I would be curious to see what happens if we just immediately call take_group_member, vs sleeping a consistent amount of time (like 100ms), etc.

Another option here might be to implement our own take_group_member with a timeout:

...
  take_group_member(:riak, 500)
...

defp take_group_member(_, timeout) when timeout =< 0 do
  :error_no_members
end
defp take_group_member(group_name, timeout) do
  case :pooler.take_group_member(group_name) do
    :error_no_members ->
      #should probably log a warning / error here so the operator knows something is wrong
      :timer.sleep(100)
      take_group_member(group_name, timeout - 100)
    pid -> pid
  end
end

:pooler.take_group_member(:riak)
pid -> pid
end
result = unquote(name)(pid, unquote_splicing(other_args))
:pooler.return_group_member(:riak, pid, :ok)
result
Expand All @@ -35,4 +40,5 @@ defmodule Riak.Pool do
defp extract_guards(else_), do: {else_, []}
defp extract_or_guards({:when, _, [left, right]}), do: [left|extract_or_guards(right)];
defp extract_or_guards(term), do: [term]

end