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

txHandler: applications rate limiter #5734

Merged

Conversation

algorandskiy
Copy link
Contributor

@algorandskiy algorandskiy commented Sep 11, 2023

Summary

Rate limit incoming apps based on app id + sender IP addr. It precedes enqueueing into backlog in TxHandler.processIncomingTxn. It takes effect only if enabled and the backlog is more than 1/2 full.
The implementation uses sharded map with sliding window limiter. The sliding window data is used to for least recently used eviction (list based).

There are two hashes used: 1) memhash64 for app to bucket mapping 2) blake2b for app id + sender caching.
Importantly, the implementation is trying to compromise on memory usage and shrinks 32 bytes blake2b to 8 bytes. It appears OK since blake2b output is uniform, and input data is salted making impossible to censor app + relay pairs.

As benchmark (below) shows there is almost no penalty (5%) on eviction when 94% of operations cause eviction.

Test Plan

  1. Unit tests: currently 98% but missing an edge case with eviction and reusing.
  2. Benchmarks
BenchmarkAppRateLimiter/multi_bucket_no_evict-8         	  179841	      6572 ns/op	         0.4306 %_drop	    1340 B/op	      16 allocs/op
BenchmarkAppRateLimiter/single_bucket_no_evict-8        	  152894	      6859 ns/op	         0.6651 %_drop	    1364 B/op	      17 allocs/op
BenchmarkAppRateLimiter/single_bucket_w_evict-8         	  147360	      7174 ns/op	         0 %_drop	    1408 B/op	      19 allocs/op
--- BENCH: BenchmarkAppRateLimiter/single_bucket_w_evict-8
    appRateLimiter_test.go:336: # evictions 15392, time 2119 us
    appRateLimiter_test.go:336: # evictions 139168, time 20589 us
  1. Real apps traffic
    Used real recorded transactions and approximated traffic by reported connected peers metric during the last high traffic event on Sep 1st 12:01 pm - 3:28pm. Got about 6M transactions and about 10k unique key pairs (no truncated hash collisions).
    Run few benchmarks by using tx messages from the generated data set and got the following data:
  • 4m11s to read the data set and feed into txHandler without rate limiting
  • 4m45s to read and apply rate limiter
  • using time.Now() gave 98% drop rate
  • using tx timestamp (with some uniform approximation) gave 92% drop rate with the same run time
  • using tx timestamp but with random order gave 1% drop rate with the same run time (different workloads, same runtime.

Visualized acceptance rate:
image

  1. Profiling / Resources
mem prof cpu prof
orig impl
memory fixes image image

Memory overhead:

  1. sync.Pool for keys/buckets gathering 0.5% alloc_space, 25% alloc_object on a heavy load.
  2. original version: 2.9% alloc_space, 38% alloc_object. Most allocs from keys/buckets gathering and go-deadlock.lock() (fixed in fix extra heap allocations when detector is disabled go-deadlock#2):
/go/pkg/mod/github.com/algorand/go-deadlock@v0.2.2/deadlock.go

  Total:      1.55GB     1.55GB (flat, cum)  0.54%
    119            .          .           		m.muId = currID 
    120            .          .           		currID++ 
    121            .          .           	} 
    122            .          .           	counterMu.Unlock() 
    123            .          .            
    124       1.55GB     1.55GB           	lock(m.mu.Lock, m, false) 
  1. Scenario1s run (same cmd as perf: upgrade go-deadlock #5760) => no perf degradation observed.
relay summary: 4893.13 TPS, 3.19s/block, tx 26.2MB/s, rx 17.4MB/s

data/appRateLimiter.go Outdated Show resolved Hide resolved
network/p2pPeer.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #5734 (eb977e2) into master (f5d901a) will increase coverage by 0.04%.
Report is 7 commits behind head on master.
The diff coverage is 92.34%.

@@            Coverage Diff             @@
##           master    #5734      +/-   ##
==========================================
+ Coverage   55.64%   55.69%   +0.04%     
==========================================
  Files         475      476       +1     
  Lines       66869    67043     +174     
==========================================
+ Hits        37209    37339     +130     
- Misses      27151    27185      +34     
- Partials     2509     2519      +10     
Files Coverage Δ
config/consensus.go 86.52% <100.00%> (+0.03%) ⬆️
config/localTemplate.go 72.02% <ø> (ø)
data/appRateLimiter.go 100.00% <100.00%> (ø)
network/wsPeer.go 71.73% <90.47%> (-1.13%) ⬇️
network/p2pPeer.go 63.63% <0.00%> (-11.37%) ⬇️
data/txHandler.go 77.08% <72.00%> (-0.91%) ⬇️

... and 8 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

data/appRateLimiter.go Outdated Show resolved Hide resolved
data/appRateLimiter.go Outdated Show resolved Hide resolved
@algorandskiy algorandskiy force-pushed the pavel/txhandler-appid-cache branch 4 times, most recently from 10582ba to ac6037f Compare September 14, 2023 23:26
data/appRateLimiter.go Outdated Show resolved Hide resolved
data/appRateLimiter.go Outdated Show resolved Hide resolved
data/appRateLimiter.go Outdated Show resolved Hide resolved
@algorandskiy
Copy link
Contributor Author

Merged master, moved new config vals to v32

data/txHandler.go Outdated Show resolved Hide resolved
data/appRateLimiter.go Outdated Show resolved Hide resolved
jasonpaulos
jasonpaulos previously approved these changes Nov 7, 2023
config/localTemplate.go Outdated Show resolved Hide resolved
Copy link
Member

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for incorporating the app ID changes

@algorandskiy algorandskiy merged commit 8e30dd4 into algorand:master Nov 9, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants