Remove webhook rate limit cache
The move to DSharpPlus makes it unnecessary, as D#+ can actually do webhook invocations on its own.
This commit is contained in:
parent
042327d4aa
commit
546cb7f97a
@ -102,21 +102,17 @@ namespace PluralKit.Bot
|
|||||||
{
|
{
|
||||||
private ILifetimeScope _services;
|
private ILifetimeScope _services;
|
||||||
private DiscordShardedClient _client;
|
private DiscordShardedClient _client;
|
||||||
private Timer _updateTimer;
|
|
||||||
private IMetrics _metrics;
|
private IMetrics _metrics;
|
||||||
private PeriodicStatCollector _collector;
|
private PeriodicStatCollector _collector;
|
||||||
private ILogger _logger;
|
private ILogger _logger;
|
||||||
private WebhookRateLimitService _webhookRateLimit;
|
|
||||||
private int _periodicUpdateCount;
|
|
||||||
private Task _periodicWorker;
|
private Task _periodicWorker;
|
||||||
|
|
||||||
public Bot(ILifetimeScope services, DiscordShardedClient client, IMetrics metrics, PeriodicStatCollector collector, ILogger logger, WebhookRateLimitService webhookRateLimit)
|
public Bot(ILifetimeScope services, DiscordShardedClient client, IMetrics metrics, PeriodicStatCollector collector, ILogger logger)
|
||||||
{
|
{
|
||||||
_services = services;
|
_services = services;
|
||||||
_client = client;
|
_client = client;
|
||||||
_metrics = metrics;
|
_metrics = metrics;
|
||||||
_collector = collector;
|
_collector = collector;
|
||||||
_webhookRateLimit = webhookRateLimit;
|
|
||||||
_logger = logger.ForContext<Bot>();
|
_logger = logger.ForContext<Bot>();
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -182,12 +178,6 @@ namespace PluralKit.Bot
|
|||||||
}
|
}
|
||||||
catch (WebSocketException) { }
|
catch (WebSocketException) { }
|
||||||
|
|
||||||
// Run webhook rate limit GC every 10 minutes
|
|
||||||
if (_periodicUpdateCount++ % 10 == 0)
|
|
||||||
{
|
|
||||||
var _ = Task.Run(() => _webhookRateLimit.GarbageCollect());
|
|
||||||
}
|
|
||||||
|
|
||||||
await _collector.CollectStats();
|
await _collector.CollectStats();
|
||||||
|
|
||||||
_logger.Information("Submitted metrics to backend");
|
_logger.Information("Submitted metrics to backend");
|
||||||
|
@ -55,7 +55,6 @@ namespace PluralKit.Bot
|
|||||||
builder.RegisterType<ProxyService>().AsSelf().SingleInstance();
|
builder.RegisterType<ProxyService>().AsSelf().SingleInstance();
|
||||||
builder.RegisterType<LogChannelService>().AsSelf().SingleInstance();
|
builder.RegisterType<LogChannelService>().AsSelf().SingleInstance();
|
||||||
builder.RegisterType<DataFileService>().AsSelf().SingleInstance();
|
builder.RegisterType<DataFileService>().AsSelf().SingleInstance();
|
||||||
builder.RegisterType<WebhookRateLimitService>().AsSelf().SingleInstance();
|
|
||||||
builder.RegisterType<WebhookExecutorService>().AsSelf().SingleInstance();
|
builder.RegisterType<WebhookExecutorService>().AsSelf().SingleInstance();
|
||||||
builder.RegisterType<WebhookCacheService>().AsSelf().SingleInstance();
|
builder.RegisterType<WebhookCacheService>().AsSelf().SingleInstance();
|
||||||
builder.RegisterType<ShardInfoService>().AsSelf().SingleInstance();
|
builder.RegisterType<ShardInfoService>().AsSelf().SingleInstance();
|
||||||
|
@ -24,13 +24,12 @@ namespace PluralKit.Bot
|
|||||||
private IDataStore _data;
|
private IDataStore _data;
|
||||||
|
|
||||||
private WebhookCacheService _webhookCache;
|
private WebhookCacheService _webhookCache;
|
||||||
private WebhookRateLimitService _webhookRateLimitCache;
|
|
||||||
|
|
||||||
private DbConnectionCountHolder _countHolder;
|
private DbConnectionCountHolder _countHolder;
|
||||||
|
|
||||||
private ILogger _logger;
|
private ILogger _logger;
|
||||||
|
|
||||||
public PeriodicStatCollector(DiscordShardedClient client, IMetrics metrics, ILogger logger, WebhookCacheService webhookCache, DbConnectionCountHolder countHolder, IDataStore data, CpuStatService cpu, WebhookRateLimitService webhookRateLimitCache)
|
public PeriodicStatCollector(DiscordShardedClient client, IMetrics metrics, ILogger logger, WebhookCacheService webhookCache, DbConnectionCountHolder countHolder, IDataStore data, CpuStatService cpu)
|
||||||
{
|
{
|
||||||
_client = client;
|
_client = client;
|
||||||
_metrics = metrics;
|
_metrics = metrics;
|
||||||
@ -38,7 +37,6 @@ namespace PluralKit.Bot
|
|||||||
_countHolder = countHolder;
|
_countHolder = countHolder;
|
||||||
_data = data;
|
_data = data;
|
||||||
_cpu = cpu;
|
_cpu = cpu;
|
||||||
_webhookRateLimitCache = webhookRateLimitCache;
|
|
||||||
_logger = logger.ForContext<PeriodicStatCollector>();
|
_logger = logger.ForContext<PeriodicStatCollector>();
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -99,7 +97,6 @@ namespace PluralKit.Bot
|
|||||||
|
|
||||||
// Other shiz
|
// Other shiz
|
||||||
_metrics.Measure.Gauge.SetValue(BotMetrics.WebhookCacheSize, _webhookCache.CacheSize);
|
_metrics.Measure.Gauge.SetValue(BotMetrics.WebhookCacheSize, _webhookCache.CacheSize);
|
||||||
_metrics.Measure.Gauge.SetValue(BotMetrics.WebhookRateLimitCacheSize, _webhookRateLimitCache.CacheSize);
|
|
||||||
|
|
||||||
stopwatch.Stop();
|
stopwatch.Stop();
|
||||||
_logger.Information("Updated metrics in {Time}", stopwatch.ElapsedDuration());
|
_logger.Information("Updated metrics in {Time}", stopwatch.ElapsedDuration());
|
||||||
|
@ -31,17 +31,15 @@ namespace PluralKit.Bot
|
|||||||
public class WebhookExecutorService
|
public class WebhookExecutorService
|
||||||
{
|
{
|
||||||
private WebhookCacheService _webhookCache;
|
private WebhookCacheService _webhookCache;
|
||||||
private WebhookRateLimitService _rateLimit;
|
|
||||||
private ILogger _logger;
|
private ILogger _logger;
|
||||||
private IMetrics _metrics;
|
private IMetrics _metrics;
|
||||||
private HttpClient _client;
|
private HttpClient _client;
|
||||||
|
|
||||||
public WebhookExecutorService(IMetrics metrics, WebhookCacheService webhookCache, ILogger logger, HttpClient client, WebhookRateLimitService rateLimit)
|
public WebhookExecutorService(IMetrics metrics, WebhookCacheService webhookCache, ILogger logger, HttpClient client)
|
||||||
{
|
{
|
||||||
_metrics = metrics;
|
_metrics = metrics;
|
||||||
_webhookCache = webhookCache;
|
_webhookCache = webhookCache;
|
||||||
_client = client;
|
_client = client;
|
||||||
_rateLimit = rateLimit;
|
|
||||||
_logger = logger.ForContext<WebhookExecutorService>();
|
_logger = logger.ForContext<WebhookExecutorService>();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1,128 +0,0 @@
|
|||||||
using System;
|
|
||||||
using System.Collections.Concurrent;
|
|
||||||
using System.Globalization;
|
|
||||||
using System.Linq;
|
|
||||||
using System.Net;
|
|
||||||
using System.Net.Http;
|
|
||||||
|
|
||||||
using DSharpPlus.Entities;
|
|
||||||
|
|
||||||
using NodaTime;
|
|
||||||
|
|
||||||
using Serilog;
|
|
||||||
|
|
||||||
namespace PluralKit.Bot
|
|
||||||
{
|
|
||||||
// Simplified rate limit handler for webhooks only, disregards most bucket functionality since scope is limited and just denies requests if too fast.
|
|
||||||
public class WebhookRateLimitService
|
|
||||||
{
|
|
||||||
private ILogger _logger;
|
|
||||||
private ConcurrentDictionary<ulong, WebhookRateLimitInfo> _info = new ConcurrentDictionary<ulong, WebhookRateLimitInfo>();
|
|
||||||
|
|
||||||
public WebhookRateLimitService(ILogger logger)
|
|
||||||
{
|
|
||||||
_logger = logger.ForContext<WebhookRateLimitService>();
|
|
||||||
}
|
|
||||||
|
|
||||||
public int CacheSize => _info.Count;
|
|
||||||
|
|
||||||
public bool TryExecuteWebhook(DiscordWebhook webhook)
|
|
||||||
{
|
|
||||||
// If we have nothing saved, just allow it (we'll save something once the response returns)
|
|
||||||
if (!_info.TryGetValue(webhook.Id, out var info)) return true;
|
|
||||||
|
|
||||||
// If we're past the reset time, allow the request and update the bucket limit
|
|
||||||
if (SystemClock.Instance.GetCurrentInstant() > info.resetTime)
|
|
||||||
{
|
|
||||||
if (!info.hasResetTimeExpired)
|
|
||||||
info.remaining = info.maxLimit;
|
|
||||||
info.hasResetTimeExpired = true;
|
|
||||||
|
|
||||||
// We can hit this multiple times if many requests are in flight before a real one gets "back", so we still
|
|
||||||
// decrement the remaining request count, this basically "blacklists" the channel given continuous spam until *one* of the requests come back with new rate limit headers
|
|
||||||
info.remaining--;
|
|
||||||
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
|
|
||||||
// If we don't have any more requests left, deny the request
|
|
||||||
if (info.remaining == 0)
|
|
||||||
{
|
|
||||||
_logger.Debug("Rate limit bucket for {Webhook} out of requests, denying request", webhook.Id);
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Otherwise, decrement the request count and allow the request
|
|
||||||
info.remaining--;
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
|
|
||||||
public void UpdateRateLimitInfo(DiscordWebhook webhook, HttpResponseMessage response)
|
|
||||||
{
|
|
||||||
var info = _info.GetOrAdd(webhook.Id, _ => new WebhookRateLimitInfo());
|
|
||||||
|
|
||||||
if (int.TryParse(GetHeader(response, "X-RateLimit-Limit"), out var limit))
|
|
||||||
info.maxLimit = limit;
|
|
||||||
|
|
||||||
// Max "safe" is way above UNIX timestamp values, and we get fractional seconds, hence the double
|
|
||||||
// but need culture/format specifiers to get around Some Locales (cough, my local PC) having different settings for decimal point...
|
|
||||||
// We also use Reset-After to avoid issues with clock desync between us and Discord's server, this way it's all relative (plus latency errors work in our favor)
|
|
||||||
if (double.TryParse(GetHeader(response, "X-RateLimit-Reset-After"), NumberStyles.Float, CultureInfo.InvariantCulture, out var resetTimestampDelta))
|
|
||||||
{
|
|
||||||
var resetTime = SystemClock.Instance.GetCurrentInstant() + Duration.FromSeconds(resetTimestampDelta);
|
|
||||||
if (resetTime > info.resetTime)
|
|
||||||
{
|
|
||||||
// Set to the *latest* reset value we have (for safety), since we rely on relative times this can jitter a bit
|
|
||||||
info.resetTime = resetTime;
|
|
||||||
info.hasResetTimeExpired = false;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if (int.TryParse(GetHeader(response, "X-RateLimit-Remaining"), out var remainingRequests))
|
|
||||||
// Overwrite a negative "we don't know" value with whatever we just got
|
|
||||||
// Otherwise, *lower* remaining requests takes precedence
|
|
||||||
if (info.remaining < 0 || remainingRequests < info.remaining)
|
|
||||||
info.remaining = remainingRequests;
|
|
||||||
|
|
||||||
_logger.Debug("Updated rate limit information for {Webhook}, bucket has {RequestsRemaining} requests remaining, reset in {ResetTime}", webhook.Id, info.remaining, info.resetTime - SystemClock.Instance.GetCurrentInstant());
|
|
||||||
|
|
||||||
if (response.StatusCode == HttpStatusCode.TooManyRequests)
|
|
||||||
{
|
|
||||||
// 429, we're *definitely* out of requests
|
|
||||||
info.remaining = 0;
|
|
||||||
_logger.Warning("Got 429 Too Many Requests when invoking webhook {Webhook}, next bucket reset in {ResetTime}", webhook.Id, info.resetTime - SystemClock.Instance.GetCurrentInstant());
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
public void GarbageCollect()
|
|
||||||
{
|
|
||||||
_logger.Information("Garbage-collecting webhook rate limit buckets...");
|
|
||||||
|
|
||||||
var collected = 0;
|
|
||||||
foreach (var channel in _info.Keys)
|
|
||||||
{
|
|
||||||
if (!_info.TryGetValue(channel, out var info)) continue;
|
|
||||||
|
|
||||||
// Remove all keys that expired more than an hour ago (and of course, haven't been reset)
|
|
||||||
if (info.resetTime < SystemClock.Instance.GetCurrentInstant() - Duration.FromHours(1))
|
|
||||||
if (_info.TryRemove(channel, out _)) collected++;
|
|
||||||
}
|
|
||||||
|
|
||||||
_logger.Information("Garbage-collected {ChannelCount} channels from the webhook rate limit buckets.", collected);
|
|
||||||
}
|
|
||||||
|
|
||||||
private string GetHeader(HttpResponseMessage response, string key)
|
|
||||||
{
|
|
||||||
var firstPair = response.Headers.FirstOrDefault(pair => pair.Key.Equals(key, StringComparison.InvariantCultureIgnoreCase));
|
|
||||||
return firstPair.Value?.FirstOrDefault(); // If key is missing, default value is null
|
|
||||||
}
|
|
||||||
|
|
||||||
private class WebhookRateLimitInfo
|
|
||||||
{
|
|
||||||
public Instant resetTime;
|
|
||||||
public bool hasResetTimeExpired;
|
|
||||||
public int remaining = -1;
|
|
||||||
public int maxLimit = 0;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
Loading…
Reference in New Issue
Block a user