Replace Hammer for rate limiting with custom ets bucket #3571
Conversation
ruslandoga
force-pushed
the
rm-hammer
branch
6 times, most recently
from
d74c955 to
9f5c573
Compare
aerosol
changed the title
|
@ruslandoga could you rebase/merge master as in #3595 ? |
|
@aerosol done ❤️ |
| {:read_concurrency, true}, | ||
| {:write_concurrency, true}, | ||
| {:decentralized_counters, true} | ||
| ]) |
There was a problem hiding this comment.
I'm looking at how Hammer's table is configured:
[
id: #Reference<0.563007245.2435186714.162753>,
decentralized_counters: false,
read_concurrency: false,
write_concurrency: false,
compressed: false,
memory: 7351569,
owner: #PID<0.3706.0>,
heir: :none,
name: :hammer_ets_buckets,
size: 389610,
node: :"plausible@plausible-app-01",
named_table: true,
type: :ordered_set,
keypos: 1,
protection: :public
]
I'm sure you've benchmarked the hell out of it, what was the difference between using set and ordered_set?
There was a problem hiding this comment.
I've only benchmarked different implementations https://github.com/ruslandoga/rate_limit, and I haven't benchmarked set vs ordered-set directly. I'll do a benchmark now.
There was a problem hiding this comment.
$ env MIX_ENV=bench mix run bench/basic.exs
Operating System: macOS
CPU Information: Apple M1
Number of Available Cores: 8
Available memory: 8 GB
Elixir 1.15.7
Erlang 26.1.2
Benchmark suite executing with the following configuration:
warmup: 2 s
time: 5 s
memory time: 0 ns
reduction time: 0 ns
parallel: 1
inputs: none specified
Estimated total run time: 14 s
Benchmarking ordered_set ...
Benchmarking set ...
Name ips average deviation median 99th %
set 5.20 M 192.26 ns ±15809.05% 166 ns 209 ns
ordered_set 3.73 M 268.42 ns ±12499.97% 209 ns 292 ns
Comparison:
set 5.20 M
ordered_set 3.73 M - 1.40x slower +76.16 nsThere was a problem hiding this comment.
| def handle_info(:clean, state) do | ||
| clean(state.table) | ||
| schedule(state.clean_period) | ||
| {:noreply, state} |
There was a problem hiding this comment.
Would it make sense to hibernate the process, since all it's doing is waiting another 10 minutes for the next cleanup?
There was a problem hiding this comment.
I haven't used process hibernation before. I'll read up on that.
There was a problem hiding this comment.
From a quick look, process hibernation forces a garbage collection (we have opts as garbage) and compacts memory usage. I think it wouldn't hurt, but at the same time the gains don't seem significant either (cleaning up opts and compacting a two-element map).
Changes
This PR replaces Hammer with a custom wrapper around ETS due to Hammer being prone to overload: ExHammer/hammer#71
I'm opening this PR for two reasons:
Tests
Changelog
Documentation
Dark mode