From 4c879b3a336aee944cf611a9d01649d62a00d622 Mon Sep 17 00:00:00 2001
From: Yuriha <121590760+yuriha-chan@users.noreply.github.com>
Date: Fri, 7 Jul 2023 23:28:27 +0900
Subject: [PATCH] perf(backend): Improve performance of FetchInstanceMetadata
 (#11128)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Perf: Avoid retries to acquire lock in fetchInstanceMetadata

* Fix

* Add Changelog

* Fix typo

* Fix lint

* 記法をMisskey式にする

* ????

* refactor
https://github.com/misskey-dev/misskey/pull/11128#pullrequestreview-1518059366

* refactor

* getいらない？

* fix

* fix

* Update CHANGELOG.md

* clean up

---------

Co-authored-by: tamaina <tamaina@hotmail.co.jp>
---
 CHANGELOG.md                                  |  1 +
 packages/backend/src/core/AppLockService.ts   |  5 --
 .../src/core/FetchInstanceMetadataService.ts  | 49 +++++++++++--------
 3 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7ed8b9d4fe..288ab07963 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -33,6 +33,7 @@
 
 ### Server
 - JSON.parse の回数を削減することで、ストリーミングのパフォーマンスを向上しました
+- 連合の配送ジョブのパフォーマンスを向上（ロック機構の見直し、Redisキャッシュの活用）
 
 ## 13.13.2
 
diff --git a/packages/backend/src/core/AppLockService.ts b/packages/backend/src/core/AppLockService.ts
index 8dd805552b..6ccaec26ba 100644
--- a/packages/backend/src/core/AppLockService.ts
+++ b/packages/backend/src/core/AppLockService.ts
@@ -32,11 +32,6 @@ export class AppLockService {
 		return this.lock(`ap-object:${uri}`, timeout);
 	}
 
-	@bindThis
-	public getFetchInstanceMetadataLock(host: string, timeout = 30 * 1000): Promise<() => void> {
-		return this.lock(`instance:${host}`, timeout);
-	}
-
 	@bindThis
 	public getChartInsertLock(lockKey: string, timeout = 30 * 1000): Promise<() => void> {
 		return this.lock(`chart-insert:${lockKey}`, timeout);
diff --git a/packages/backend/src/core/FetchInstanceMetadataService.ts b/packages/backend/src/core/FetchInstanceMetadataService.ts
index 9de633350b..96ee77d056 100644
--- a/packages/backend/src/core/FetchInstanceMetadataService.ts
+++ b/packages/backend/src/core/FetchInstanceMetadataService.ts
@@ -3,8 +3,6 @@ import { Inject, Injectable } from '@nestjs/common';
 import { JSDOM } from 'jsdom';
 import tinycolor from 'tinycolor2';
 import type { Instance } from '@/models/entities/Instance.js';
-import type { InstancesRepository } from '@/models/index.js';
-import { AppLockService } from '@/core/AppLockService.js';
 import type Logger from '@/logger.js';
 import { DI } from '@/di-symbols.js';
 import { LoggerService } from '@/core/LoggerService.js';
@@ -12,6 +10,7 @@ import { HttpRequestService } from '@/core/HttpRequestService.js';
 import { bindThis } from '@/decorators.js';
 import { FederatedInstanceService } from '@/core/FederatedInstanceService.js';
 import type { DOMWindow } from 'jsdom';
+import * as Redis from 'ioredis';
 
 type NodeInfo = {
 	openRegistrations?: unknown;
@@ -37,33 +36,43 @@ export class FetchInstanceMetadataService {
 	private logger: Logger;
 
 	constructor(
-		@Inject(DI.instancesRepository)
-		private instancesRepository: InstancesRepository,
-
-		private appLockService: AppLockService,
 		private httpRequestService: HttpRequestService,
 		private loggerService: LoggerService,
 		private federatedInstanceService: FederatedInstanceService,
+		@Inject(DI.redis)
+		private redisClient: Redis.Redis,
 	) {
 		this.logger = this.loggerService.getLogger('metadata', 'cyan');
 	}
 
+	@bindThis
+	public async tryLock(host: string): Promise<boolean> {
+		const mutex = await this.redisClient.set(`fetchInstanceMetadata:mutex:${host}`, '1', 'GET');
+		return mutex !== '1';
+	}
+
+	@bindThis
+	public unlock(host: string): Promise<'OK'> {
+		return this.redisClient.set(`fetchInstanceMetadata:mutex:${host}`, '0');
+	}
+
 	@bindThis
 	public async fetchInstanceMetadata(instance: Instance, force = false): Promise<void> {
-		const unlock = await this.appLockService.getFetchInstanceMetadataLock(instance.host);
-	
-		if (!force) {
-			const _instance = await this.instancesRepository.findOneBy({ host: instance.host });
-			const now = Date.now();
-			if (_instance && _instance.infoUpdatedAt && (now - _instance.infoUpdatedAt.getTime() < 1000 * 60 * 60 * 24)) {
-				unlock();
-				return;
-			}
-		}
-	
-		this.logger.info(`Fetching metadata of ${instance.host} ...`);
-	
+		const host = instance.host;
+		// Acquire mutex to ensure no parallel runs
+		if (!await this.tryLock(host)) return;
 		try {
+			if (!force) {
+				const _instance = await this.federatedInstanceService.fetch(host);
+				const now = Date.now();
+				if (_instance && _instance.infoUpdatedAt && (now - _instance.infoUpdatedAt.getTime() < 1000 * 60 * 60 * 24)) {
+					// unlock at the finally caluse
+					return;
+				}
+			}
+	
+			this.logger.info(`Fetching metadata of ${instance.host} ...`);
+	
 			const [info, dom, manifest] = await Promise.all([
 				this.fetchNodeinfo(instance).catch(() => null),
 				this.fetchDom(instance).catch(() => null),
@@ -104,7 +113,7 @@ export class FetchInstanceMetadataService {
 		} catch (e) {
 			this.logger.error(`Failed to update metadata of ${instance.host}: ${e}`);
 		} finally {
-			unlock();
+			await this.unlock(host);
 		}
 	}