Page MenuHomePhabricator

Implement parallel connection limit for querying ORES
Closed, ResolvedPublic

Description

4 simultaneous connections per client (IP + User-Agent) should be good.

Relatedly, this is done in nginx for dump downloads. See https://phabricator.wikimedia.org/diffusion/OPUP/browse/production/modules/dumps/templates/nginx.dumps.conf.erb

Event Timeline

Halfak created this task.Oct 24 2016, 6:43 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 24 2016, 6:43 PM
Halfak triaged this task as Medium priority.Oct 27 2016, 2:42 PM
Halfak moved this task from Untriaged to Maintenance/cleanup on the Scoring-platform-team board.
Tgr added a subscriber: Tgr.Feb 7 2017, 3:08 AM

Presumably internal IPs should be exempt from this, and the API should set XFF headers when proxying requests?

Halfak added a comment.Feb 7 2017, 3:25 PM

Yeah. My thoughts too.

Tgr added a comment.Feb 13 2017, 9:49 PM

Per T137962#2447946, "Generic ratelimiting (e.g. per client IP) and other similar protection measures for these clusters has been pushed off for post-varnish4" so that's not an option right now.

If we rely on some naive frontend implementation (e.g. ngx_http_limit_conn_module) on the web worker or MW API nodes then we end up with an N connections per node limit instead of a global one. Are IPs assigned to nodes via some deterministic hashing, or just round-robin? In the first case, what would happen when nodes are added / removed? (ie. are we using consistent hashing?)

Maybe it could be done in the ORES load balancer (if it's based on proxying and not DNS lookups). That wouldn't be a proper parallel connection limit but a limit on number of connections initiated per time unit, but under normal operation ORES does not have long-lived connections so that should be close enough.

Alternatively the throttling could be implemented in the app code, using some shared resource such as Redis. That's a probably a performance hit, even if a very small one, so it's doable but less ideal than using some existing varnish/nginx/whatever functionality.

Ladsgroup claimed this task.Nov 7 2018, 6:04 PM
Ladsgroup added a subscriber: Ladsgroup.

Bah, this is doneeeeee

Restricted Application added a project: User-Ladsgroup. · View Herald TranscriptNov 7 2018, 6:04 PM
Ladsgroup closed this task as Resolved.Nov 7 2018, 6:05 PM
Ladsgroup moved this task from Active to Done on the Scoring-platform-team (Current) board.