From ef4cc80c63fab283b2a546ed06e23fd51dd6b107 Mon Sep 17 00:00:00 2001 From: Fleny Date: Tue, 30 Jul 2024 20:04:32 +0200 Subject: [PATCH] fix(bot,gateway): Fix (some) resharding bugs (#3792) * Improve logging, keep shard collections separated * revert to erroring in getSessionInfo. Fill the value in createBot * removed useless if * remove same number of shards check Co-authored-by: Awesome Stickz * Update manager.ts * revert shard.events.message assignment for the spread * use new Collection instead of .clear() + for-of * Use fixed formula and note in the comment the inaccuracy * Apply suggestions from code review Co-authored-by: Awesome Stickz * Add a remark to shardsFullPercentage to let the user know of the inaccuracy * Update comment and add a clearInterval The clearInterval is not strictly needed, however it is better to ensure there is always one and only one internal running at a given point * revert check for all shards online It was bugged as gateway.resharding.shards gets populated overtime and not instantly --------- Co-authored-by: Awesome Stickz --- packages/bot/src/bot.ts | 6 +++ packages/gateway/src/manager.ts | 81 +++++++++++++++++++++++---------- 2 files changed, 62 insertions(+), 25 deletions(-) diff --git a/packages/bot/src/bot.ts b/packages/bot/src/bot.ts index 75b9fafb4..6dec89cdd 100644 --- a/packages/bot/src/bot.ts +++ b/packages/bot/src/bot.ts @@ -88,6 +88,12 @@ export function createBot(options: CreateBotOptions): Bot { if (!options.gateway?.lastShardId && !options.gateway?.totalShards) bot.gateway.lastShardId = bot.gateway.connection.shards - 1 } + if (!bot.gateway.resharding.getSessionInfo) { + bot.gateway.resharding.getSessionInfo = async () => { + return await bot.rest.getGatewayBot() + } + } + await bot.gateway.spawnShards() }, diff --git a/packages/gateway/src/manager.ts b/packages/gateway/src/manager.ts index d140e2788..98f69abcd 100644 --- a/packages/gateway/src/manager.ts +++ b/packages/gateway/src/manager.ts @@ -57,7 +57,7 @@ export function createGatewayManager(options: CreateGatewayManagerOptions): Gate resharding: { enabled: true, shardsFullPercentage: 80, - checkInterval: 28800000, + checkInterval: 28800000, // 8 hours shards: new Collection(), pendingShards: new Collection(), async getSessionInfo() { @@ -66,24 +66,44 @@ export function createGatewayManager(options: CreateGatewayManagerOptions): Gate ) }, async checkIfReshardingIsNeeded() { - gateway.logger.warn(`[Resharding] Checking if resharding is needed.`) - // Resharding is disabled. - if (!gateway.resharding.enabled) return { needed: false } - gateway.logger.warn(`[Resharding] Resharding is enabled.`) + gateway.logger.warn('[Resharding] Checking if resharding is needed.') + + if (!gateway.resharding.enabled) { + gateway.logger.debug('[Resharding] Resharding is disabled.') + + return { needed: false } + } + + gateway.logger.warn('[Resharding] Resharding is enabled.') const sessionInfo = await gateway.resharding.getSessionInfo() - gateway.logger.warn(`[Resharding] Session info retrieved.`) + + gateway.logger.debug(`[Resharding] Session info retrieved: ${JSON.stringify(sessionInfo)}`) + // Don't have enough identify limits to try resharding - if (sessionInfo.sessionStartLimit.remaining < sessionInfo.shards) return { needed: false, info: sessionInfo } - gateway.logger.warn(`[Resharding] Able to reshard, checking whether necessary now.`) + if (sessionInfo.sessionStartLimit.remaining < sessionInfo.shards) { + gateway.logger.warn('[Resharding] Not enough identify limits to try resharding.') + + return { needed: false, info: sessionInfo } + } + + gateway.logger.warn('[Resharding] Able to reshard, checking whether necessary now.') // 2500 is the max amount of guilds a single shard can handle // 1000 is the amount of guilds discord uses to determine how many shards to recommend. // This algo helps check if your bot has grown enough to reshard. - const percentage = ((2500 * sessionInfo.shards) / (gateway.totalShards * 1000)) * 100 + // While this is imprecise as discord changes the recommended number of shard every 1000 guilds it is good enough + // The alternative is to store the guild count for each shard and require the Guilds intent for `GUILD_CREATE` and `GUILD_DELETE` events + const percentage = (sessionInfo.shards / ((gateway.totalShards * 2500) / 1000)) * 100 + // Less than necessary% being used so do nothing - if (percentage < gateway.resharding.shardsFullPercentage) return { needed: false, info: sessionInfo } - gateway.logger.warn(`[Resharding] Resharding is needed.`) + if (percentage < gateway.resharding.shardsFullPercentage) { + gateway.logger.warn('[Resharding] Resharding not needed.') + + return { needed: false, info: sessionInfo } + } + + gateway.logger.warn('[Resharding] Resharding is needed.') return { needed: true, info: sessionInfo } }, @@ -137,6 +157,7 @@ export function createGatewayManager(options: CreateGatewayManagerOptions): Gate } }, }, + logger: gateway.logger, requestIdentify: async () => { await gateway.identify(shardId) }, @@ -170,24 +191,21 @@ export function createGatewayManager(options: CreateGatewayManagerOptions): Gate }) }) }, - async updateGuildsShardId(guildIds, shardId) { - gateway.logger.debug( - `[Resharding] Updating the following guild ids shard to #${shardId}: ${guildIds.join(', ')}. Override this function to update your cache if you need to.`, - ) - }, + async updateGuildsShardId(_guildIds, _shardId) {}, async shardIsPending(shard) { // Save this in pending at the moment, until all shards are online gateway.resharding.pendingShards.set(shard.id, shard) gateway.logger.warn(`[Resharding] Shard #${shard.id} is now pending`) + // Check if all shards are now online. - if (gateway.lastShardId - gateway.firstShardId + 1 > gateway.resharding.pendingShards.size) return + if (gateway.lastShardId - gateway.firstShardId >= gateway.resharding.pendingShards.size) return + gateway.logger.warn(`[Resharding] All shards are now online.`) // New shards start processing events - for (const shard of gateway.resharding.pendingShards.values()) { + for (const shard of gateway.resharding.shards.values()) { for (const event in options.events) { - // @ts-expect-error fixing this seems complex and doesnt seem worth it, feel free to fix if you wish - shard.events[event] = options.events[event] + shard.events[event as keyof ShardEvents] = options.events[event as keyof ShardEvents] as (...args: unknown[]) => unknown } } @@ -200,9 +218,9 @@ export function createGatewayManager(options: CreateGatewayManagerOptions): Gate ...shard.events, message: async function (_, message) { // Member checks need to continue but others can stop - if (message.t !== 'GUILD_MEMBERS_CHUNK') return - // Process only the chunking events - oldHandler?.(shard, message) + if (message.t === 'GUILD_MEMBERS_CHUNK') { + oldHandler?.(shard, message) + } }, } } @@ -210,10 +228,15 @@ export function createGatewayManager(options: CreateGatewayManagerOptions): Gate gateway.logger.warn(`[Resharding] Shutting down old shards.`) // Close old shards await gateway.shutdown(ShardSocketCloseCodes.Resharded, 'Resharded!', false) + gateway.logger.warn(`[Resharding] Completed.`) // Replace old shards - gateway.shards = gateway.resharding.shards + gateway.shards = new Collection(gateway.resharding.shards) + + // Clear our collections and keep only one reference to the shards, the one in gateway.shards + gateway.resharding.shards.clear() + gateway.resharding.pendingShards.clear() }, }, @@ -302,6 +325,9 @@ export function createGatewayManager(options: CreateGatewayManagerOptions): Gate // Check and reshard automatically if auto resharding is enabled. if (gateway.resharding.enabled && gateway.resharding.checkInterval !== -1) { + // It is better to ensure there is always only one + clearInterval(gateway.resharding.checkIntervalId) + gateway.resharding.checkIntervalId = setInterval(async () => { const reshardingInfo = await gateway.resharding.checkIfReshardingIsNeeded() @@ -312,7 +338,7 @@ export function createGatewayManager(options: CreateGatewayManagerOptions): Gate async shutdown(code, reason, clearReshardingInterval = true) { gateway.shards.forEach((shard) => shard.close(code, reason)) - if (clearReshardingInterval && gateway.resharding.checkIntervalId) clearInterval(gateway.resharding.checkIntervalId) + if (clearReshardingInterval) clearInterval(gateway.resharding.checkIntervalId) await delay(5000) }, @@ -640,6 +666,11 @@ export interface GatewayManager extends Required { enabled: boolean /** * The % of how full a shard is when resharding should be triggered. + * + * @remarks + * We use discord recommended shard value to get an **approximation** of the shard full percentage to compare with this value so the bot may not reshard at the exact percentage provided but may reshard when it is a bit higher than the provided percentage. + * For accurate calculation, you may override the `checkIfReshardingIsNeeded` function + * * @default 80 as in 80% */ shardsFullPercentage: number