Closed Bug 929647 Opened 7 years ago Closed 7 years ago

web form throttling

Categories

(Input Graveyard :: Submission, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: willkg, Assigned: willkg)

References

Details

(Whiteboard: u=user c=feedback p=3 s=input.2013q4)

We currently throttle incoming responses via the Input API. We should additionally throttle incoming responses via the web form.

This bug covers figuring out all the details:

1. how to implement throttling

2. what levels to throttle at

3. what happens when a user who is entering in feedback triggers the throttle (e.g. at a conference where other people have issued feedback)
I'm pretty sure this is a non-trivial bug that will take about a week to implement. It may also involve strings changes depending on how we want to deal with the ui portion. Adding whiteboard data accordingly.
Priority: -- → P1
Whiteboard: u=user c=feedback p=3 s=input.2013q4
I just talked to Mike about this since he implemented throttling in SUMO.

SUMO uses django-ratelimit <https://github.com/jsocol/django-ratelimit> which (as you can see) is written by James Socol. That seems to do everything I'd want it to do, plus it's probably easier to fix issues if there are any. I'm doing a pass on the project, issues and pull requests to see how it's faring right now. Then I'll use that.

I don't know what to do about levels to throttle at. I suspect we can crib from SUMO.

I need to make sure we're logging how often someone gets throttled so we can keep track of whether we're hosing more people than we want to be. Just like with CSRF changes we made a while back.

For now, I think I'm just going to ignore the case when someone gets throttled and do nothing. We can figure out later if that's ok. If it's not, then we have some non-trivial work to do to make the feedback form save state and be cleverer about things.
Action plan:

1. use django-ratelimit (waiting on some fixes to land)
2. throttle with same levels as the input api (100/hour)
3. add some statsd code to keep track of how many throttlings we're doing
4. when someone hits the throttle threshold, just redirect to the thank you page

Note: Write up a bug to fix that last bit when we redo the feedback form.
Assignee: nobody → willkg
Status: NEW → ASSIGNED
Just waiting on a django-ratelimit 0.4 release.
Landed in master in https://github.com/mozilla/fjord/commit/c426735

Pushed to stage and prod.

However, I don't see anything Input-related on graphite, so I opened bug #933280 to figure out what's going on there.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Something is wrong with the throttling code and it caused a massive drop off in responses. I deployed an earlier commit to production. The commit is still in the master branch (I didn't reverse it).

If I had to hazard a guess, it's throttling on the ip address of the load balancer and not the ip address of the client. Reopening this to fix that.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Bug #933712 covers the throttling problem.
Fix in PR https://github.com/mozilla/fjord/pull/166

I'm blocking on bug #933280 because I need to be able to see what's going on on the servers before I try another attempt pushing this.
Depends on: 933280
Fixes landed in master in https://github.com/mozilla/fjord/commit/7a260af

I'm going to push this to stage and prod. statsd/graphite is capturing and showing data again, so I'll keep an eye on it over it for the rest of the day and if things look like they're going south, I'll undo it again.
Pushed to stage and prod just now. I'll monitor this for a day and if after that it looks good, I'll mark it FIXED.
Everything continues to look good. We haven't had any throttling kick in, yet. I don't expect any throttling to happen except in egregious circumstances, so that's expected.

Marking this as FIXED.
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Product: Input → Input Graveyard
You need to log in before you can comment on or make changes to this bug.