Skip to content

Commit

Permalink
Revert "feat: concurrent connections limiter (#696)" (#750)
Browse files Browse the repository at this point in the history
This reverts commit de2f305.
  • Loading branch information
nlwstein authored Feb 9, 2024
1 parent de2f305 commit 1598989
Show file tree
Hide file tree
Showing 16 changed files with 16 additions and 385 deletions.
4 changes: 1 addition & 3 deletions apps/api_accounts/lib/api_accounts/key.ex
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ defmodule ApiAccounts.Key do
field(:requested_date, :datetime)
field(:approved, :boolean, default: false)
field(:locked, :boolean, default: false)
field(:static_concurrent_limit, :integer)
field(:streaming_concurrent_limit, :integer)
field(:daily_limit, :integer)
field(:rate_request_pending, :boolean, default: false)
field(:api_version, :string)
Expand All @@ -30,7 +28,7 @@ defmodule ApiAccounts.Key do
@doc false
def changeset(struct, params \\ %{}) do
fields = ~w(
created requested_date approved locked static_concurrent_limit streaming_concurrent_limit daily_limit rate_request_pending api_version description allowed_domains
created requested_date approved locked daily_limit rate_request_pending api_version description allowed_domains
)a
cast(struct, params, fields)
end
Expand Down
18 changes: 1 addition & 17 deletions apps/api_web/config/config.exs
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,7 @@ config :api_web, :rate_limiter,
limiter: ApiWeb.RateLimiter.ETS,
max_anon_per_interval: 5_000,
max_registered_per_interval: 100_000,
wait_time_ms: 0,
connection_opts: [
coder: Memcache.Coder.JSON
]

config :api_web, :rate_limiter_concurrent,
enabled: false,
memcache: false,
log_statistics: true,
limit_users: false,
# How many seconds tolerated when calculating whether a connection is still open
# 45 - 30 (see ApiWeb.EventStream.Initialize's timeout value) gives us a buffer of 15 seconds:
heartbeat_tolerance: 45,
# Default concurrent connections - these can be overridden on a per-key basis in the admin UI:
max_anon_static: 5,
max_registered_streaming: 10,
max_registered_static: 20
wait_time_ms: 0

config :api_web, ApiWeb.Plugs.ModifiedSinceHandler, check_caller: false

Expand Down
2 changes: 0 additions & 2 deletions apps/api_web/config/dev.exs
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,3 @@ config :logger, :console, format: "[$level] $message\n", level: :debug
# Do not configure such in production as keeping
# and calculating stacktraces is usually expensive.
config :phoenix, :stacktrace_depth, 20

config :api_web, :rate_limiter_concurrent, enabled: false, memcache: false
4 changes: 0 additions & 4 deletions apps/api_web/config/prod.exs
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,6 @@ config :ehmon, :report_mf, {:ehmon, :info_report}

config :logster, :filter_parameters, ~w(password password_confirm)

config :api_web, :rate_limiter_concurrent,
enabled: true,
memcache: true

config :api_web, :rate_limiter,
clear_interval: 60_000,
max_anon_per_interval: 20,
Expand Down
4 changes: 0 additions & 4 deletions apps/api_web/config/test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,3 @@ config :recaptcha,

# Print only warnings and errors during test
config :logger, level: :warn

config :api_web, :rate_limiter_concurrent,
enabled: false,
memcache: false
2 changes: 0 additions & 2 deletions apps/api_web/lib/api_web.ex
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ defmodule ApiWeb do
# no cover
children = [
# Start the endpoint when the application starts
ApiWeb.RateLimiter.Memcache.Supervisor,
ApiWeb.RateLimiter,
ApiWeb.RateLimiter.RateLimiterConcurrent,
{RequestTrack, [name: ApiWeb.RequestTrack]},
ApiWeb.EventStream.Supervisor,
ApiWeb.Endpoint,
Expand Down
1 change: 0 additions & 1 deletion apps/api_web/lib/api_web/api_controller_helpers.ex
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ defmodule ApiWeb.ApiControllerHelpers do
plug(:split_include)
plug(ApiWeb.Plugs.ModifiedSinceHandler, caller: __MODULE__)
plug(ApiWeb.Plugs.RateLimiter)
plug(ApiWeb.Plugs.RateLimiterConcurrent)

def index(conn, params), do: ApiControllerHelpers.index(__MODULE__, conn, params)

Expand Down
15 changes: 0 additions & 15 deletions apps/api_web/lib/api_web/event_stream.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ defmodule ApiWeb.EventStream do
import Plug.Conn
alias __MODULE__.Supervisor
alias ApiWeb.Plugs.CheckForShutdown
alias ApiWeb.RateLimiter.RateLimiterConcurrent
require Logger

@enforce_keys [:conn, :pid, :timeout]
Expand Down Expand Up @@ -54,13 +53,6 @@ defmodule ApiWeb.EventStream do

@spec hibernate_loop(state) :: Plug.Conn.t()
def hibernate_loop(state) do
if Map.has_key?(state.conn.assigns, :api_user) do
# Update the concurrent rate limit cache to ensure any flushing doesn't impact long-running connections:
RateLimiterConcurrent.add_lock(state.conn.assigns.api_user, self(), true)
else
Logger.warn("#{__MODULE__} missing_api_user - cannot rate limit!")
end

case receive_result(state) do
{:continue, state} ->
:proc_lib.hibernate(__MODULE__, :hibernate_loop, [state])
Expand Down Expand Up @@ -138,13 +130,6 @@ defmodule ApiWeb.EventStream do
end

defp unsubscribe(state) do
if Map.has_key?(state.conn.assigns, :api_user) do
# clean up our concurrent connections lock:
RateLimiterConcurrent.remove_lock(state.conn.assigns.api_user, self(), true)
else
Logger.warn("#{__MODULE__} missing_api_user - cannot rate limit!")
end

# consume any extra messages received after unsubscribing
receive do
{:events, _} ->
Expand Down
70 changes: 0 additions & 70 deletions apps/api_web/lib/api_web/plugs/rate_limiter_concurrent.ex

This file was deleted.

12 changes: 6 additions & 6 deletions apps/api_web/lib/api_web/rate_limiter/memcache.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ defmodule ApiWeb.RateLimiter.Memcache do
"""
@behaviour ApiWeb.RateLimiter.Limiter
alias ApiWeb.RateLimiter.Memcache.Supervisor
use GenServer

@impl ApiWeb.RateLimiter.Limiter
def start_link(opts) do
GenServer.start_link(__MODULE__, opts, name: __MODULE__)
end
clear_interval_ms = Keyword.fetch!(opts, :clear_interval)
clear_interval = div(clear_interval_ms, 1000)

connection_opts =
[ttl: clear_interval * 2] ++ ApiWeb.config(RateLimiter.Memcache, :connection_opts)

@impl true
def init(opts) do
{:ok, opts}
Supervisor.start_link(connection_opts)
end

@impl ApiWeb.RateLimiter.Limiter
Expand Down
33 changes: 7 additions & 26 deletions apps/api_web/lib/api_web/rate_limiter/memcache/supervisor.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,17 @@ defmodule ApiWeb.RateLimiter.Memcache.Supervisor do
"""
@worker_count 5
@registry_name __MODULE__.Registry
@rate_limit_config Application.compile_env!(:api_web, :rate_limiter)

use Agent

def start_link(_) do
clear_interval_ms = Keyword.fetch!(@rate_limit_config, :clear_interval)
clear_interval = div(clear_interval_ms, 1000)
connection_opts_config = Keyword.fetch!(@rate_limit_config, :connection_opts)
connection_opts = [ttl: clear_interval * 2] ++ connection_opts_config

def start_link(connection_opts) do
registry = {Registry, keys: :unique, name: @registry_name}

children =
if memcache_required?() do
workers =
for i <- 1..@worker_count do
Supervisor.child_spec({Memcache, [connection_opts, [name: worker_name(i)]]}, id: i)
end

[registry | workers]
else
[registry]
workers =
for i <- 1..@worker_count do
Supervisor.child_spec({Memcache, [connection_opts, [name: worker_name(i)]]}, id: i)
end

children = [registry | workers]

Supervisor.start_link(
children,
strategy: :rest_for_one,
Expand All @@ -44,13 +31,7 @@ defmodule ApiWeb.RateLimiter.Memcache.Supervisor do
{:via, Registry, {@registry_name, index}}
end

defp memcache_required? do
(ApiWeb.RateLimiter.RateLimiterConcurrent.enabled?() and
ApiWeb.RateLimiter.RateLimiterConcurrent.memcache?()) or
ApiWeb.config(:rate_limiter, :limiter) == ApiWeb.RateLimiter.Memcache
end

def random_child do
defp random_child do
worker_name(:rand.uniform(@worker_count))
end
end
Loading

0 comments on commit 1598989

Please sign in to comment.