Fix rate limits

[BenCookie95]

Summary

Convert QueriesPerMinute to float64 before dividing by 60. When QA were testing the rate limit with a quota below 60 it would set the bucket size to 0. I also noticed some small issue with duplicated notifications where the modified time coming from Google is slightly different to the last activity timestamp.

Testing

I recommend setting the Google Quota limits to 24. Plugin configs:
QueriesPerMinute: 18
BurstSize: 6

[davidkrauser]

LGTM 👍

[AayushChaudhary0001]

While testing this PR, it was working fine for the values already added in the description. But on decreasing the values, I was facing an issue which was as I tried to trigger multiple events(adding comments) during a definite time interval, I was rate limited which was correct because it exceeded. But as the time passed and as per our discussion, the bucket should fill after 60 seconds and we should receive the next notification, instead I was rate limited for next 4-5 minutes and no new notification was received even after 10 minutes.
The scenario was that I added 7 comments at 16:13 and received the notification for only first 3 comments. The error was presented in the logs till 16:17 and no further notification was received.
Limits set were as follows:-
Google cloud:- 16
Queries per minute:- 10
Max burst size allowed:- 6

Notifications:-

Server logs:-

{"timestamp":"2024-12-11 16:13:45.679 +05:30","level":"error","msg":"Failed to fetch Google Drive changes","caller":"app/plugin_api.go:1011","plugin_id":"com.mattermost-community.plugin-google-drive","err":"rate: Wait(n=1) would exceed context deadline","userID":"89i8o3wkmfbrtpabduuymf6cec"}

{"timestamp":"2024-12-11 16:14:29.544 +05:30","level":"error","msg":"Failed to fetch Google Drive changes","caller":"app/plugin_api.go:1011","plugin_id":"com.mattermost-community.plugin-google-drive","err":"rate: Wait(n=1) would exceed context deadline","userID":"89i8o3wkmfbrtpabduuymf6cec"}

{"timestamp":"2024-12-11 16:15:12.624 +05:30","level":"error","msg":"Failed to fetch Google Drive changes","caller":"app/plugin_api.go:1011","plugin_id":"com.mattermost-community.plugin-google-drive","err":"rate: Wait(n=1) would exceed context deadline","userID":"89i8o3wkmfbrtpabduuymf6cec"}

{"timestamp":"2024-12-11 16:17:18.588 +05:30","level":"error","msg":"Failed to fetch Google Drive changes","caller":"app/plugin_api.go:1011","plugin_id":"com.mattermost-community.plugin-google-drive","err":"rate: Wait(n=1) would exceed context deadline","userID":"89i8o3wkmfbrtpabduuymf6cec"}

[BenCookie95]

While testing this PR, it was working fine for the values already added in the description. But on decreasing the values, I was facing an issue which was as I tried to trigger multiple events(adding comments) during a definite time interval, I was rate limited which was correct because it exceeded. But as the time passed and as per our discussion, the bucket should fill after 60 seconds and we should receive the next notification, instead I was rate limited for next 4-5 minutes and no new notification was received even after 10 minutes. The scenario was that I added 7 comments at 16:13 and received the notification for only first 3 comments. The error was presented in the logs till 16:17 and no further notification was received. Limits set were as follows:- Google cloud:- 16 Queries per minute:- 10 Max burst size allowed:- 6

Notifications:-

Server logs:-

{"timestamp":"2024-12-11 16:13:45.679 +05:30","level":"error","msg":"Failed to fetch Google Drive changes","caller":"app/plugin_api.go:1011","plugin_id":"com.mattermost-community.plugin-google-drive","err":"rate: Wait(n=1) would exceed context deadline","userID":"89i8o3wkmfbrtpabduuymf6cec"}

{"timestamp":"2024-12-11 16:14:29.544 +05:30","level":"error","msg":"Failed to fetch Google Drive changes","caller":"app/plugin_api.go:1011","plugin_id":"com.mattermost-community.plugin-google-drive","err":"rate: Wait(n=1) would exceed context deadline","userID":"89i8o3wkmfbrtpabduuymf6cec"}

{"timestamp":"2024-12-11 16:15:12.624 +05:30","level":"error","msg":"Failed to fetch Google Drive changes","caller":"app/plugin_api.go:1011","plugin_id":"com.mattermost-community.plugin-google-drive","err":"rate: Wait(n=1) would exceed context deadline","userID":"89i8o3wkmfbrtpabduuymf6cec"}

{"timestamp":"2024-12-11 16:17:18.588 +05:30","level":"error","msg":"Failed to fetch Google Drive changes","caller":"app/plugin_api.go:1011","plugin_id":"com.mattermost-community.plugin-google-drive","err":"rate: Wait(n=1) would exceed context deadline","userID":"89i8o3wkmfbrtpabduuymf6cec"}

@AayushChaudhary0001 Thanks for bringing this to our attention.

I spent a lot of time today digging into why this is happening at the lower rate limit levels and I finally have an explanation. So for example when you have a lower queries per minute of 10, it will regenerate a token for the bucket every 6 seconds. The burst size is 6 which means the bucket has a maximum capacity of 6.

Let say you generate 20 notifications for this 1 user at once. The webhook timeout is 60 seconds and there is a cluster mutex per user which means that 1 request gets through while 19 others are waiting. That 1 request might use 3 tokens to fulfil it's notification. Now there are 3 tokens in the bucket and the rate limiter has started to regenerate tokens. The next request gets through and does the same thing etc etc. Now lets say there are still 14 requests sitting and waiting for the mutex lock after we have processed the others, the bucket is now empty. The rate limiter does this thing where it will look at the request timeout and see if there is enough time left in the request in order for it to regenerate a new token. If it determines the request will timeout before that time, it will throw an error. So now this happens for the 14 tokens. The problem is that Google will try again because we told them the request has failed, we receive the 14 requests again 2 seconds later. The problem is that our artificially low rate limit numbers take 6 seconds to regenerate a single token so we end up with this bucket that cannot refill and a backlog of Google notifications.

That's why you were seeing those issues with the lower numbers. When we are using a more realistic rate limit of 12000 queries per minute, we will regenerate 200 tokens per second which means we won't end up with a large backlog like that. I' ok to pass this test in it's current state considering it was working with the higher limits

[AayushChaudhary0001]

@BenCookie95 Thanks for the explanation, it makes sense and we can pass that test case. Just wanted to confirm that do I need to test it again since there is a new commit added after you have added the comment.

[BenCookie95]

@BenCookie95 Thanks for the explanation, it makes sense and we can pass that test case. Just wanted to confirm that do I need to test it again since there is a new commit added after you have added the comment.

@AayushChaudhary0001 if you're able to test in the next hour or 2 then yes, if not then I will merge this because we need to cut a release. I will do a sanity test myself before merging anyway