From 2d148907424336584b09481b8558beb855a99c61 Mon Sep 17 00:00:00 2001 From: "Christopher J. Bottaro" Date: Sat, 1 Jul 2017 16:48:32 -0500 Subject: [PATCH 1/2] Set the serialization bit when writing --- lib/memcache.ex | 21 ++++++++++---- lib/memcache/connection.ex | 59 ++++++++++++++++++++++++++++++++------ 2 files changed, 67 insertions(+), 13 deletions(-) diff --git a/lib/memcache.ex b/lib/memcache.ex index a223c2e..d7eab32 100644 --- a/lib/memcache.ex +++ b/lib/memcache.ex @@ -110,13 +110,24 @@ defmodule Memcache do """ @spec start_link(Keyword.t, Keyword.t) :: GenServer.on_start def start_link(connection_options \\ [], options \\ []) do - extra_opts = [:ttl, :namespace, :key_coder, :coder] - connection_options = Keyword.merge(@default_opts, connection_options) - |> Keyword.update!(:coder, &normalize_coder/1) - state = connection_options |> Keyword.take(extra_opts) |> Enum.into(%{}) - {:ok, pid} = Connection.start_link(Keyword.drop(connection_options, extra_opts), options) + memcache_options = [:ttl, :namespace, :key_coder] + shared_options = [:coder] + + connection_options = @default_opts + |> Keyword.merge(connection_options) + |> Keyword.update!(:coder, &normalize_coder/1) + + state = connection_options + |> Keyword.take(memcache_options ++ shared_options) + |> Enum.into(%{}) + + connection_options = Keyword.drop(connection_options, memcache_options) + + {:ok, pid} = Connection.start_link(connection_options, options) + state = Map.put(state, :connection, pid) Registry.associate(pid, state) + {:ok, pid} end diff --git a/lib/memcache/connection.ex b/lib/memcache/connection.ex index d42a830..1b1c22d 100644 --- a/lib/memcache/connection.ex +++ b/lib/memcache/connection.ex @@ -54,7 +54,9 @@ defmodule Memcache.Connection do """ @spec start_link(Keyword.t, Keyword.t) :: GenServer.on_start def start_link(connection_options \\ [], options \\ []) do - connection_options = with_defaults(connection_options) + connection_options = connection_options + |> with_defaults + |> with_flags Connection.start_link(__MODULE__, connection_options, options) end @@ -70,6 +72,19 @@ defmodule Memcache.Connection do |> Keyword.update!(:hostname, (&if is_binary(&1), do: String.to_char_list(&1), else: &1)) end + # For Dalli compatibility, we need to set the first bit of "flags" to 1 if + # a serializer (coder) is used when setting keys. We're going to precompute + # flags based on if a coder is used and store it in our state. See serialize/4 + # for the other half of this. + defp with_flags(opts) do + {coder, opts} = Keyword.pop(opts, :coder) + flags = case coder do + {Memcache.Coder.Raw, _} -> 0 + _ -> 1 + end + Keyword.put(opts, :flags, flags) + end + @doc """ Executes the command with the given args @@ -244,7 +259,8 @@ defmodule Memcache.Connection do end defp send_and_receive(%State{ sock: sock } = s, from, command, args, opts) do - packet = serialize(command, args) + flags = Keyword.get(s.opts, :flags, 0) + packet = serialize(command, args, 0, flags) case :gen_tcp.send(sock, packet) do :ok -> s = enqueue_receiver(s, from) @@ -255,7 +271,8 @@ defmodule Memcache.Connection do end defp send_and_receive_quiet(%State{ sock: sock } = s, from, commands) do - { packet, commands, i } = Enum.reduce(commands, { [], [], 1 }, &accumulate_commands/2) + flags = Keyword.get(s.opts, :flags, 0) + { packet, commands, i } = Enum.reduce(commands, { [], [], 1 }, &accumulate_commands(&1, &2, flags)) packet = [packet | serialize(:NOOP, [], i)] case :gen_tcp.send(sock, packet) do :ok -> @@ -271,11 +288,11 @@ defmodule Memcache.Connection do %{state| receiver_queue: receiver_queue} end - defp accumulate_commands({ command, args }, { packet, commands, i }) do - { [packet | serialize(command, args, i)], [{ i, command, args, %{cas: false} } | commands], i + 1 } + defp accumulate_commands({ command, args }, { packet, commands, i }, flags) do + { [packet | serialize(command, args, i, flags)], [{ i, command, args, %{cas: false} } | commands], i + 1 } end - defp accumulate_commands({ command, args, options }, { packet, commands, i }) do - { [packet | serialize(command, args, i)], [{ i, command, args, %{cas: Keyword.get(options, :cas, false)}} | commands], i + 1 } + defp accumulate_commands({ command, args, options }, { packet, commands, i }, flags) do + { [packet | serialize(command, args, i, flags)], [{ i, command, args, %{cas: Keyword.get(options, :cas, false)}} | commands], i + 1 } end defp get_backoff(s) do @@ -352,7 +369,33 @@ defmodule Memcache.Connection do end end - defp serialize(command, args, opaque \\ 0) do + defp serialize(command, args), do: serialize(command, args, 0) + + defp serialize(command, args, opaque) do apply(Protocol, :to_binary, [command | [opaque | args]]) end + + # For Dalli compatibility, we need to set the first bit of flags to 1 when + # using a coder (serializer) with the following commands. We've stored flags + # in our state and now just need to use it when serializing the command. + defp serialize(command, args, opaque, flags) + when command == :SET + when command == :SETQ + when command == :ADD + when command == :ADDQ + when command == :REPLACE + when command == :REPLACEQ do + + # to_binary for the above commands can default up to three args: cas, expiry, flags. + # And since flags is the last arg, we have to account for that here. + args = case length(args) do + 2 -> args ++ [0, 0, flags] + 3 -> args ++ [0, flags] + 4 -> args ++ [flags] + end + + serialize(command, args, opaque) + end + + defp serialize(command, args, opaque, _flags), do: serialize(command, args, opaque) end From 5cd4e0d97f004d2bccb089eaa74420d6dd67c569 Mon Sep 17 00:00:00 2001 From: "Christopher J. Bottaro" Date: Sat, 1 Jul 2017 17:14:59 -0500 Subject: [PATCH 2/2] Move calculating flags into Memcache and out of Memcache.Connection --- lib/memcache.ex | 33 +++++++++++++++++++-------------- lib/memcache/connection.ex | 17 +---------------- 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/lib/memcache.ex b/lib/memcache.ex index d7eab32..cdc24f7 100644 --- a/lib/memcache.ex +++ b/lib/memcache.ex @@ -110,27 +110,32 @@ defmodule Memcache do """ @spec start_link(Keyword.t, Keyword.t) :: GenServer.on_start def start_link(connection_options \\ [], options \\ []) do - memcache_options = [:ttl, :namespace, :key_coder] - shared_options = [:coder] + extra_opts = [:ttl, :namespace, :key_coder, :coder] + connection_options = Keyword.merge(@default_opts, connection_options) + |> Keyword.update!(:coder, &normalize_coder/1) + state = connection_options |> Keyword.take(extra_opts) |> Enum.into(%{}) - connection_options = @default_opts - |> Keyword.merge(connection_options) - |> Keyword.update!(:coder, &normalize_coder/1) - - state = connection_options - |> Keyword.take(memcache_options ++ shared_options) - |> Enum.into(%{}) - - connection_options = Keyword.drop(connection_options, memcache_options) - - {:ok, pid} = Connection.start_link(connection_options, options) + {:ok, pid} = connection_options + |> add_flags + |> Keyword.drop(extra_opts) + |> Connection.start_link(options) state = Map.put(state, :connection, pid) Registry.associate(pid, state) - + {:ok, pid} end + # When we're using a serializer/coder, then we need to let Connection know to + # set the serialization bit when writing keys. For Dalli compat. + defp add_flags(opts) do + flags = case opts[:coder] do + {Memcache.Coder.Raw, _} -> 0 + _ -> 1 + end + Keyword.put(opts, :flags, flags) + end + @doc """ Closes the connection to the memcached server. """ diff --git a/lib/memcache/connection.ex b/lib/memcache/connection.ex index 1b1c22d..2993260 100644 --- a/lib/memcache/connection.ex +++ b/lib/memcache/connection.ex @@ -54,9 +54,7 @@ defmodule Memcache.Connection do """ @spec start_link(Keyword.t, Keyword.t) :: GenServer.on_start def start_link(connection_options \\ [], options \\ []) do - connection_options = connection_options - |> with_defaults - |> with_flags + connection_options = with_defaults(connection_options) Connection.start_link(__MODULE__, connection_options, options) end @@ -72,19 +70,6 @@ defmodule Memcache.Connection do |> Keyword.update!(:hostname, (&if is_binary(&1), do: String.to_char_list(&1), else: &1)) end - # For Dalli compatibility, we need to set the first bit of "flags" to 1 if - # a serializer (coder) is used when setting keys. We're going to precompute - # flags based on if a coder is used and store it in our state. See serialize/4 - # for the other half of this. - defp with_flags(opts) do - {coder, opts} = Keyword.pop(opts, :coder) - flags = case coder do - {Memcache.Coder.Raw, _} -> 0 - _ -> 1 - end - Keyword.put(opts, :flags, flags) - end - @doc """ Executes the command with the given args