1543612 - Add some sanity limits to throttle ping uploads

Status: RESOLVED FIXED

(Data Platform and Tools :: Glean: SDK, defect, P1)

Description

[Alessio Placitelli [:Dexter]]

Glean is currently not preventing attempts to flood servers with pings: for example, with events, whenever we reach the maximum capacity we generate and send a new ping.

While this is currently not a problem, we might want to consider rate-limiting uploads to 1 ping type per minute or something like that.

Comment 1

[Alessio Placitelli [:Dexter]]

(In reply to Alessio Placitelli [:Dexter] from comment #0)

While this is currently not a problem, we might want to consider rate-limiting uploads to 1 ping type per minute or something like that.

Of course this should not apply to pings sent using the GleanDebugActivity (maybe only to tagged ones?).

Comment 2

[Beatriz Rizental [:brizental]]

I started looking into this and I have some open questions regarding what I should take into consideration while implementing:

• Should this throttle time be persisted? Is it a concern if the chronometer just zeroes everytime the upload manager is reinitialized?
• I was thinking about just adding this "once per minute" limitation to the calling of get_upload_task, but that means that say upload has failed, if the client asks for a new task it will still have to wait the one minute before getting another one. I am not really sure how I could go around this in a simple manner, if it is indeed a concern.

Comment 3

[Alessio Placitelli [:Dexter]]

(In reply to Beatriz Rizental from comment #2)

I started looking into this and I have some open questions regarding what I should take into consideration while implementing:

• Should this throttle time be persisted? Is it a concern if the chronometer just zeroes everytime the upload manager is reinitialized?

What do you mean here?

• I was thinking about just adding this "once per minute" limitation to the calling of get_upload_task, but that means that say upload has failed, if the client asks for a new task it will still have to wait the one minute before getting another one. I am not really sure how I could go around this in a simple manner, if it is indeed a concern.

Please note that what we really want to achieve is rate-limiting ping upload. You can use the Firefox rate limiting as a guidelines. We want at most 10 pings per minute.

Does this help?

Comment 4

[Beatriz Rizental [:brizental]]

(In reply to Alessio Placitelli [:Dexter] from comment #3)

(In reply to Beatriz Rizental from comment #2)

I started looking into this and I have some open questions regarding what I should take into consideration while implementing:

• Should this throttle time be persisted? Is it a concern if the chronometer just zeroes everytime the upload manager is reinitialized?

What do you mean here?

I mean, if the client sends a ping, then closes the app, then immediatelly reopens it. Do we need to know that it just sent a ping a second ago? Or is this enough of an edge case that I can ignore it?

• I was thinking about just adding this "once per minute" limitation to the calling of get_upload_task, but that means that say upload has failed, if the client asks for a new task it will still have to wait the one minute before getting another one. I am not really sure how I could go around this in a simple manner, if it is indeed a concern.

Please note that what we really want to achieve is rate-limiting ping upload. You can use the Firefox rate limiting as a guidelines. We want at most 10 pings per minute.

Does this help?

The example helps, yes.

Comment 5

[Alessio Placitelli [:Dexter]]

(In reply to Beatriz Rizental from comment #4)

(In reply to Alessio Placitelli [:Dexter] from comment #3)

(In reply to Beatriz Rizental from comment #2)

I started looking into this and I have some open questions regarding what I should take into consideration while implementing:

• Should this throttle time be persisted? Is it a concern if the chronometer just zeroes everytime the upload manager is reinitialized?

What do you mean here?

I mean, if the client sends a ping, then closes the app, then immediatelly reopens it. Do we need to know that it just sent a ping a second ago? Or is this enough of an edge case that I can ignore it?

No, we can ignore this edge case for now.

Comment 6

[Beatriz Rizental [:brizental]]

Attachment: GitHub Pull Request