From f3eca0b5cfff9b22f0ac90b65c2c979a9ef0b56d Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sat, 24 May 2025 18:43:43 -0400 Subject: populate block fields when registering a new instance --- .../backend/src/core/FederatedInstanceService.ts | 40 ++++++++++------------ 1 file changed, 18 insertions(+), 22 deletions(-) (limited to 'packages/backend/src/core/FederatedInstanceService.ts') diff --git a/packages/backend/src/core/FederatedInstanceService.ts b/packages/backend/src/core/FederatedInstanceService.ts index 3f7ed99348..e31d802e8c 100644 --- a/packages/backend/src/core/FederatedInstanceService.ts +++ b/packages/backend/src/core/FederatedInstanceService.ts @@ -5,9 +5,9 @@ import { Inject, Injectable, OnApplicationShutdown } from '@nestjs/common'; import * as Redis from 'ioredis'; -import { QueryFailedError } from 'typeorm'; +import { DataSource, QueryFailedError } from 'typeorm'; import type { InstancesRepository } from '@/models/_.js'; -import type { MiInstance } from '@/models/Instance.js'; +import { MiInstance } from '@/models/Instance.js'; import { MemoryKVCache, RedisKVCache } from '@/misc/cache.js'; import { IdService } from '@/core/IdService.js'; import { DI } from '@/di-symbols.js'; @@ -26,6 +26,9 @@ export class FederatedInstanceService implements OnApplicationShutdown { @Inject(DI.instancesRepository) private instancesRepository: InstancesRepository, + @Inject(DI.db) + private readonly db: DataSource, + private utilityService: UtilityService, private idService: IdService, ) { @@ -55,34 +58,27 @@ export class FederatedInstanceService implements OnApplicationShutdown { const cached = await this.federatedInstanceCache.get(host); if (cached) return cached; - const index = await this.instancesRepository.findOneBy({ host }); + return await this.db.transaction(async tem => { + let index = await tem.findOneBy(MiInstance, { host }); - if (index == null) { - let i; - try { - i = await this.instancesRepository.insertOne({ + if (index == null) { + await tem.insert(MiInstance, { id: this.idService.gen(), host, firstRetrievedAt: new Date(), + isBlocked: this.utilityService.isBlockedHost(host), + isSilenced: this.utilityService.isSilencedHost(host), + isMediaSilenced: this.utilityService.isMediaSilencedHost(host), + isAllowListed: this.utilityService.isAllowListedHost(host), + isBubbled: this.utilityService.isBubbledHost(host), }); - } catch (e: unknown) { - if (e instanceof QueryFailedError) { - if (isDuplicateKeyValueError(e)) { - i = await this.instancesRepository.findOneBy({ host }); - } - } - - if (i == null) { - throw e; - } + + index = await tem.findOneByOrFail(MiInstance, { host }); } - this.federatedInstanceCache.set(host, i); - return i; - } else { - this.federatedInstanceCache.set(host, index); + await this.federatedInstanceCache.set(host, index); return index; - } + }); } @bindThis -- cgit v1.2.3-freya From 3e7ab07b3caa91c9caff7aeb072cf26f2e13ffc9 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 25 May 2025 08:44:45 -0400 Subject: avoid race conditions in meta / instance insert --- .../backend/src/core/FederatedInstanceService.ts | 30 ++++++-------- packages/backend/src/core/MetaService.ts | 48 +++++++++++----------- 2 files changed, 37 insertions(+), 41 deletions(-) (limited to 'packages/backend/src/core/FederatedInstanceService.ts') diff --git a/packages/backend/src/core/FederatedInstanceService.ts b/packages/backend/src/core/FederatedInstanceService.ts index e31d802e8c..662c7f7f2d 100644 --- a/packages/backend/src/core/FederatedInstanceService.ts +++ b/packages/backend/src/core/FederatedInstanceService.ts @@ -5,15 +5,13 @@ import { Inject, Injectable, OnApplicationShutdown } from '@nestjs/common'; import * as Redis from 'ioredis'; -import { DataSource, QueryFailedError } from 'typeorm'; import type { InstancesRepository } from '@/models/_.js'; -import { MiInstance } from '@/models/Instance.js'; +import type { MiInstance } from '@/models/Instance.js'; import { MemoryKVCache, RedisKVCache } from '@/misc/cache.js'; import { IdService } from '@/core/IdService.js'; import { DI } from '@/di-symbols.js'; import { UtilityService } from '@/core/UtilityService.js'; import { bindThis } from '@/decorators.js'; -import { isDuplicateKeyValueError } from '@/misc/is-duplicate-key-value-error.js'; @Injectable() export class FederatedInstanceService implements OnApplicationShutdown { @@ -26,9 +24,6 @@ export class FederatedInstanceService implements OnApplicationShutdown { @Inject(DI.instancesRepository) private instancesRepository: InstancesRepository, - @Inject(DI.db) - private readonly db: DataSource, - private utilityService: UtilityService, private idService: IdService, ) { @@ -58,11 +53,11 @@ export class FederatedInstanceService implements OnApplicationShutdown { const cached = await this.federatedInstanceCache.get(host); if (cached) return cached; - return await this.db.transaction(async tem => { - let index = await tem.findOneBy(MiInstance, { host }); - - if (index == null) { - await tem.insert(MiInstance, { + let index = await this.instancesRepository.findOneBy({ host }); + if (index == null) { + await this.instancesRepository.createQueryBuilder('instance') + .insert() + .values({ id: this.idService.gen(), host, firstRetrievedAt: new Date(), @@ -71,14 +66,15 @@ export class FederatedInstanceService implements OnApplicationShutdown { isMediaSilenced: this.utilityService.isMediaSilencedHost(host), isAllowListed: this.utilityService.isAllowListedHost(host), isBubbled: this.utilityService.isBubbledHost(host), - }); + }) + .orIgnore() + .execute(); - index = await tem.findOneByOrFail(MiInstance, { host }); - } + index = await this.instancesRepository.findOneByOrFail({ host }); + } - await this.federatedInstanceCache.set(host, index); - return index; - }); + await this.federatedInstanceCache.set(host, index); + return index; } @bindThis diff --git a/packages/backend/src/core/MetaService.ts b/packages/backend/src/core/MetaService.ts index 5b6ee8920e..b4ccfec4cc 100644 --- a/packages/backend/src/core/MetaService.ts +++ b/packages/backend/src/core/MetaService.ts @@ -14,6 +14,7 @@ import type { GlobalEvents } from '@/core/GlobalEventService.js'; import { FeaturedService } from '@/core/FeaturedService.js'; import { MiInstance } from '@/models/Instance.js'; import { diffArrays } from '@/misc/diff-arrays.js'; +import type { MetasRepository } from '@/models/_.js'; import type { OnApplicationShutdown } from '@nestjs/common'; @Injectable() @@ -28,6 +29,9 @@ export class MetaService implements OnApplicationShutdown { @Inject(DI.db) private db: DataSource, + @Inject(DI.metasRepository) + private readonly metasRepository: MetasRepository, + private featuredService: FeaturedService, private globalEventService: GlobalEventService, ) { @@ -69,35 +73,31 @@ export class MetaService implements OnApplicationShutdown { public async fetch(noCache = false): Promise { if (!noCache && this.cache) return this.cache; - return await this.db.transaction(async transactionalEntityManager => { - // 過去のバグでレコードが複数出来てしまっている可能性があるので新しいIDを優先する - const metas = await transactionalEntityManager.find(MiMeta, { + // 過去のバグでレコードが複数出来てしまっている可能性があるので新しいIDを優先する + let meta = await this.metasRepository.findOne({ + order: { + id: 'DESC', + }, + }); + + if (!meta) { + await this.metasRepository.createQueryBuilder('meta') + .insert() + .values({ + id: 'x', + }) + .orIgnore() + .execute(); + + meta = await this.metasRepository.findOneOrFail({ order: { id: 'DESC', }, }); + } - const meta = metas[0]; - - if (meta) { - this.cache = meta; - return meta; - } else { - // metaが空のときfetchMetaが同時に呼ばれるとここが同時に呼ばれてしまうことがあるのでフェイルセーフなupsertを使う - const saved = await transactionalEntityManager - .upsert( - MiMeta, - { - id: 'x', - }, - ['id'], - ) - .then((x) => transactionalEntityManager.findOneByOrFail(MiMeta, x.identifiers[0])); - - this.cache = saved; - return saved; - } - }); + this.cache = meta; + return meta; } @bindThis -- cgit v1.2.3-freya From 7385f30903111bbedc2ed8f83f40a63407edcd08 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 25 May 2025 12:16:34 -0400 Subject: clear federatedInstanceCache when meta host lists change --- .../backend/src/core/FederatedInstanceService.ts | 71 ++++++++++++++-------- .../backend/src/core/entities/UserEntityService.ts | 2 +- packages/backend/src/misc/cache.ts | 9 +++ 3 files changed, 56 insertions(+), 26 deletions(-) (limited to 'packages/backend/src/core/FederatedInstanceService.ts') diff --git a/packages/backend/src/core/FederatedInstanceService.ts b/packages/backend/src/core/FederatedInstanceService.ts index 662c7f7f2d..eb634b1d68 100644 --- a/packages/backend/src/core/FederatedInstanceService.ts +++ b/packages/backend/src/core/FederatedInstanceService.ts @@ -5,21 +5,24 @@ import { Inject, Injectable, OnApplicationShutdown } from '@nestjs/common'; import * as Redis from 'ioredis'; -import type { InstancesRepository } from '@/models/_.js'; +import type { InstancesRepository, MiMeta } from '@/models/_.js'; import type { MiInstance } from '@/models/Instance.js'; -import { MemoryKVCache, RedisKVCache } from '@/misc/cache.js'; +import { MemoryKVCache } from '@/misc/cache.js'; import { IdService } from '@/core/IdService.js'; import { DI } from '@/di-symbols.js'; import { UtilityService } from '@/core/UtilityService.js'; import { bindThis } from '@/decorators.js'; +import type { GlobalEvents } from '@/core/GlobalEventService.js'; +import { Serialized } from '@/types.js'; +import { diffArrays } from '@/misc/diff-arrays.js'; @Injectable() export class FederatedInstanceService implements OnApplicationShutdown { - public federatedInstanceCache: RedisKVCache; + private readonly federatedInstanceCache: MemoryKVCache; constructor( - @Inject(DI.redis) - private redisClient: Redis.Redis, + @Inject(DI.redisForSub) + private redisForSub: Redis.Redis, @Inject(DI.instancesRepository) private instancesRepository: InstancesRepository, @@ -27,30 +30,15 @@ export class FederatedInstanceService implements OnApplicationShutdown { private utilityService: UtilityService, private idService: IdService, ) { - this.federatedInstanceCache = new RedisKVCache(this.redisClient, 'federatedInstance', { - lifetime: 1000 * 60 * 30, // 30m - memoryCacheLifetime: 1000 * 60 * 3, // 3m - fetcher: (key) => this.instancesRepository.findOneBy({ host: key }), - toRedisConverter: (value) => JSON.stringify(value), - fromRedisConverter: (value) => { - const parsed = JSON.parse(value); - if (parsed == null) return null; - return { - ...parsed, - firstRetrievedAt: new Date(parsed.firstRetrievedAt), - latestRequestReceivedAt: parsed.latestRequestReceivedAt ? new Date(parsed.latestRequestReceivedAt) : null, - infoUpdatedAt: parsed.infoUpdatedAt ? new Date(parsed.infoUpdatedAt) : null, - notRespondingSince: parsed.notRespondingSince ? new Date(parsed.notRespondingSince) : null, - }; - }, - }); + this.federatedInstanceCache = new MemoryKVCache(1000 * 60 * 3); // 3m + this.redisForSub.on('message', this.onMessage); } @bindThis public async fetchOrRegister(host: string): Promise { host = this.utilityService.toPuny(host); - const cached = await this.federatedInstanceCache.get(host); + const cached = this.federatedInstanceCache.get(host); if (cached) return cached; let index = await this.instancesRepository.findOneBy({ host }); @@ -73,7 +61,7 @@ export class FederatedInstanceService implements OnApplicationShutdown { index = await this.instancesRepository.findOneByOrFail({ host }); } - await this.federatedInstanceCache.set(host, index); + this.federatedInstanceCache.set(host, index); return index; } @@ -81,7 +69,7 @@ export class FederatedInstanceService implements OnApplicationShutdown { public async fetch(host: string): Promise { host = this.utilityService.toPuny(host); - const cached = await this.federatedInstanceCache.get(host); + const cached = this.federatedInstanceCache.get(host); if (cached !== undefined) return cached; const index = await this.instancesRepository.findOneBy({ host }); @@ -109,8 +97,35 @@ export class FederatedInstanceService implements OnApplicationShutdown { this.federatedInstanceCache.set(result.host, result); } + private syncCache(before: Serialized, after: Serialized): void { + const changed = + hasDiff(before?.blockedHosts, after.blockedHosts) || + hasDiff(before?.silencedHosts, after.silencedHosts) || + hasDiff(before?.mediaSilencedHosts, after.mediaSilencedHosts) || + hasDiff(before?.federationHosts, after.federationHosts) || + hasDiff(before?.bubbleInstances, after.bubbleInstances); + + if (changed) { + // We have to clear the whole thing, otherwise subdomains won't be synced. + this.federatedInstanceCache.clear(); + } + } + + @bindThis + private async onMessage(_: string, data: string): Promise { + const obj = JSON.parse(data); + + if (obj.channel === 'internal') { + const { type, body } = obj.message as GlobalEvents['internal']['payload']; + if (type === 'metaUpdated') { + this.syncCache(body.before, body.after); + } + } + } + @bindThis public dispose(): void { + this.redisForSub.off('message', this.onMessage); this.federatedInstanceCache.dispose(); } @@ -119,3 +134,9 @@ export class FederatedInstanceService implements OnApplicationShutdown { this.dispose(); } } + +function hasDiff(before: string[] | null | undefined, after: string[] | null | undefined): boolean { + const { added, removed } = diffArrays(before, after); + return added.length > 0 || removed.length > 0; +} + diff --git a/packages/backend/src/core/entities/UserEntityService.ts b/packages/backend/src/core/entities/UserEntityService.ts index 56506a5fa4..feddb8fa94 100644 --- a/packages/backend/src/core/entities/UserEntityService.ts +++ b/packages/backend/src/core/entities/UserEntityService.ts @@ -609,7 +609,7 @@ export class UserEntityService implements OnModuleInit { requireSigninToViewContents: user.requireSigninToViewContents === false ? undefined : true, makeNotesFollowersOnlyBefore: user.makeNotesFollowersOnlyBefore ?? undefined, makeNotesHiddenBefore: user.makeNotesHiddenBefore ?? undefined, - instance: user.host ? this.federatedInstanceService.federatedInstanceCache.fetch(user.host).then(instance => instance ? { + instance: user.host ? this.federatedInstanceService.fetch(user.host).then(instance => instance ? { name: instance.name, softwareName: instance.softwareName, softwareVersion: instance.softwareVersion, diff --git a/packages/backend/src/misc/cache.ts b/packages/backend/src/misc/cache.ts index 48b8f43678..a6ab96c189 100644 --- a/packages/backend/src/misc/cache.ts +++ b/packages/backend/src/misc/cache.ts @@ -308,8 +308,17 @@ export class MemoryKVCache { } } + /** + * Removes all entries from the cache, but does not dispose it. + */ + @bindThis + public clear(): void { + this.cache.clear(); + } + @bindThis public dispose(): void { + this.clear(); clearInterval(this.gcIntervalHandle); } -- cgit v1.2.3-freya From 35dfde838be2dd50ee37d420f87a0cac917e621c Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 25 May 2025 12:34:09 -0400 Subject: add function diffArraysSimple for more efficient change detection --- .../backend/src/core/FederatedInstanceService.ts | 18 +++------ packages/backend/src/misc/diff-arrays.ts | 44 ++++++++++++++++++++++ packages/backend/test/unit/misc/diff-arrays.ts | 40 +++++++++++++++++++- 3 files changed, 89 insertions(+), 13 deletions(-) (limited to 'packages/backend/src/core/FederatedInstanceService.ts') diff --git a/packages/backend/src/core/FederatedInstanceService.ts b/packages/backend/src/core/FederatedInstanceService.ts index eb634b1d68..34df10f0ff 100644 --- a/packages/backend/src/core/FederatedInstanceService.ts +++ b/packages/backend/src/core/FederatedInstanceService.ts @@ -14,7 +14,7 @@ import { UtilityService } from '@/core/UtilityService.js'; import { bindThis } from '@/decorators.js'; import type { GlobalEvents } from '@/core/GlobalEventService.js'; import { Serialized } from '@/types.js'; -import { diffArrays } from '@/misc/diff-arrays.js'; +import { diffArrays, diffArraysSimple } from '@/misc/diff-arrays.js'; @Injectable() export class FederatedInstanceService implements OnApplicationShutdown { @@ -99,11 +99,11 @@ export class FederatedInstanceService implements OnApplicationShutdown { private syncCache(before: Serialized, after: Serialized): void { const changed = - hasDiff(before?.blockedHosts, after.blockedHosts) || - hasDiff(before?.silencedHosts, after.silencedHosts) || - hasDiff(before?.mediaSilencedHosts, after.mediaSilencedHosts) || - hasDiff(before?.federationHosts, after.federationHosts) || - hasDiff(before?.bubbleInstances, after.bubbleInstances); + diffArraysSimple(before?.blockedHosts, after.blockedHosts) || + diffArraysSimple(before?.silencedHosts, after.silencedHosts) || + diffArraysSimple(before?.mediaSilencedHosts, after.mediaSilencedHosts) || + diffArraysSimple(before?.federationHosts, after.federationHosts) || + diffArraysSimple(before?.bubbleInstances, after.bubbleInstances); if (changed) { // We have to clear the whole thing, otherwise subdomains won't be synced. @@ -134,9 +134,3 @@ export class FederatedInstanceService implements OnApplicationShutdown { this.dispose(); } } - -function hasDiff(before: string[] | null | undefined, after: string[] | null | undefined): boolean { - const { added, removed } = diffArrays(before, after); - return added.length > 0 || removed.length > 0; -} - diff --git a/packages/backend/src/misc/diff-arrays.ts b/packages/backend/src/misc/diff-arrays.ts index b3879cc996..b50ca1d4f7 100644 --- a/packages/backend/src/misc/diff-arrays.ts +++ b/packages/backend/src/misc/diff-arrays.ts @@ -56,3 +56,47 @@ export function diffArrays(dataBefore: T[] | null | undefined, dataAfter: T[] // data NEITHER before nor after => no change return { added: [], removed: [] }; } + +/** + * Checks for any difference between two snapshots of data. + * Null, undefined, and empty arrays are supported, and duplicate values are ignored. + * The inputs are treated as un-ordered, so a re-ordering of the same data will NOT be considered a change. + * @param dataBefore Array containing data before the change + * @param dataAfter Array containing data after the change + */ +export function diffArraysSimple(dataBefore: T[] | null | undefined, dataAfter: T[] | null | undefined): boolean { + const before = dataBefore ? new Set(dataBefore) : null; + const after = dataAfter ? new Set(dataAfter) : null; + + if (before?.size && after?.size) { + // different size => changed + if (before.size !== after.size) return true; + + // removed => changed + for (const host of before) { + // delete operation removes duplicates to speed up the "after" loop + if (!after.delete(host)) { + return true; + } + } + + // added => changed + for (const host of after) { + if (!before.has(host)) { + return true; + } + } + + // identical values => no change + return false; + } + + // before and NOT after => change + if (before?.size) return true; + + // after and NOT before => change + if (after?.size) return true; + + // NEITHER before nor after => no change + return false; +} diff --git a/packages/backend/test/unit/misc/diff-arrays.ts b/packages/backend/test/unit/misc/diff-arrays.ts index a2dee50652..b6db5e2eca 100644 --- a/packages/backend/test/unit/misc/diff-arrays.ts +++ b/packages/backend/test/unit/misc/diff-arrays.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -import { diffArrays } from '@/misc/diff-arrays.js'; +import { diffArrays, diffArraysSimple } from '@/misc/diff-arrays.js'; describe(diffArrays, () => { it('should return empty result when both inputs are null', () => { @@ -51,3 +51,41 @@ describe(diffArrays, () => { expect(result.removed).toEqual(['b', 'd']); }); }); + +describe(diffArraysSimple, () => { + it('should return false when both inputs are null', () => { + const result = diffArraysSimple(null, null); + expect(result).toBe(false); + }); + + it('should return false when both inputs are empty', () => { + const result = diffArraysSimple([], []); + expect(result).toBe(false); + }); + + it('should return true when before is populated and after is empty', () => { + const result = diffArraysSimple([1, 2, 3], []); + expect(result).toBe(true); + }); + + it('should return true when before is empty and after is populated', () => { + const result = diffArraysSimple([], [1, 2, 3]); + expect(result).toBe(true); + }); + + it('should return true when values have changed', () => { + const result = diffArraysSimple( + ['a', 'a', 'b', 'c'], + ['a', 'b', 'c', 'd'], + ); + expect(result).toBe(true); + }); + + it('should return false when values have not changed', () => { + const result = diffArraysSimple( + ['a', 'a', 'b', 'c'], + ['a', 'b', 'c', 'c'], + ); + expect(result).toBe(false); + }); +}); -- cgit v1.2.3-freya