From 5c2315679135a5971c4be4c557a8cbc3162ca7a3 Mon Sep 17 00:00:00 2001 From: Rico Kahler Date: Fri, 30 Aug 2024 09:54:15 -0500 Subject: [PATCH] feat(cli): allow setting `--max-fetch-concurrency` to prevent stalled validators --- .../cli/actions/validation/validateAction.ts | 11 +++++ .../actions/validation/validateDocuments.ts | 8 ++-- .../documents/validateDocumentsCommand.ts | 3 +- .../cli/threads/validateDocuments.ts | 3 ++ .../src/core/validation/validateDocument.ts | 44 ++++++++++++++++--- 5 files changed, 58 insertions(+), 11 deletions(-) diff --git a/packages/sanity/src/_internal/cli/actions/validation/validateAction.ts b/packages/sanity/src/_internal/cli/actions/validation/validateAction.ts index 1a6c390de393..175ae43c73b3 100644 --- a/packages/sanity/src/_internal/cli/actions/validation/validateAction.ts +++ b/packages/sanity/src/_internal/cli/actions/validation/validateAction.ts @@ -18,6 +18,7 @@ interface ValidateFlags { 'file'?: string 'level'?: 'error' | 'warning' | 'info' 'max-custom-validation-concurrency'?: number + 'max-fetch-concurrency'?: number 'yes'?: boolean 'y'?: boolean } @@ -103,6 +104,15 @@ export default async function validateAction( throw new Error(`'--max-custom-validation-concurrency' must be an integer.`) } + const maxFetchConcurrency = flags['max-fetch-concurrency'] + if ( + maxFetchConcurrency && + typeof maxFetchConcurrency !== 'number' && + !Number.isInteger(maxFetchConcurrency) + ) { + throw new Error(`'--max-fetch-concurrency' must be an integer.`) + } + const clientConfig: Partial = { ...apiClient({ requireUser: true, @@ -140,6 +150,7 @@ export default async function validateAction( workDir, level, maxCustomValidationConcurrency, + maxFetchConcurrency, ndjsonFilePath, reporter: (worker) => { const reporter = diff --git a/packages/sanity/src/_internal/cli/actions/validation/validateDocuments.ts b/packages/sanity/src/_internal/cli/actions/validation/validateDocuments.ts index 7f7a2ef27ee2..aa2be78083db 100644 --- a/packages/sanity/src/_internal/cli/actions/validation/validateDocuments.ts +++ b/packages/sanity/src/_internal/cli/actions/validation/validateDocuments.ts @@ -11,8 +11,6 @@ import { } from '../../threads/validateDocuments' import {createReceiver, type WorkerChannelReceiver} from '../../util/workerChannels' -const DEFAULT_MAX_CUSTOM_VALIDATION_CONCURRENCY = 5 - export interface ValidateDocumentsOptions { level?: 'error' | 'warning' | 'info' workspace?: string @@ -23,6 +21,7 @@ export interface ValidateDocumentsOptions { dataset?: string // override ndjsonFilePath?: string maxCustomValidationConcurrency?: number + maxFetchConcurrency?: number reporter?: (worker: WorkerChannelReceiver) => TReturn } @@ -72,6 +71,7 @@ export function validateDocuments(options: ValidateDocumentsOptions): unknown { reporter = defaultReporter, level, maxCustomValidationConcurrency, + maxFetchConcurrency, ndjsonFilePath, } = options @@ -100,8 +100,8 @@ export function validateDocuments(options: ValidateDocumentsOptions): unknown { projectId, level, ndjsonFilePath, - maxCustomValidationConcurrency: - maxCustomValidationConcurrency ?? DEFAULT_MAX_CUSTOM_VALIDATION_CONCURRENCY, + maxCustomValidationConcurrency, + maxFetchConcurrency, } satisfies ValidateDocumentsWorkerData, // eslint-disable-next-line no-process-env env: process.env, diff --git a/packages/sanity/src/_internal/cli/commands/documents/validateDocumentsCommand.ts b/packages/sanity/src/_internal/cli/commands/documents/validateDocumentsCommand.ts index 7ea450f3ab43..03e28738425b 100644 --- a/packages/sanity/src/_internal/cli/commands/documents/validateDocumentsCommand.ts +++ b/packages/sanity/src/_internal/cli/commands/documents/validateDocumentsCommand.ts @@ -11,6 +11,7 @@ Options --format The output format used to print the found validation markers and report progress. --level The minimum level reported out. Defaults to warning. --max-custom-validation-concurrency Specify how many custom validators can run concurrently. Defaults to 5. + --max-fetch-concurrency Specify how many \`client.fetch\` requests are allow concurrency at once. Defaults to 25. Examples # Validates all documents in a Sanity project with more than one workspace @@ -20,7 +21,7 @@ Examples sanity documents validate --workspace default --dataset staging # Save the results of the report into a file - sanity documents validate > report.txt + sanity documents validate --yes > report.txt # Report out info level validation markers too sanity documents validate --level info diff --git a/packages/sanity/src/_internal/cli/threads/validateDocuments.ts b/packages/sanity/src/_internal/cli/threads/validateDocuments.ts index 981f09305cd9..41716d260fd7 100644 --- a/packages/sanity/src/_internal/cli/threads/validateDocuments.ts +++ b/packages/sanity/src/_internal/cli/threads/validateDocuments.ts @@ -43,6 +43,7 @@ export interface ValidateDocumentsWorkerData { ndjsonFilePath?: string level?: ValidationMarker['level'] maxCustomValidationConcurrency?: number + maxFetchConcurrency?: number } /** @internal */ @@ -79,6 +80,7 @@ const { projectId, level, maxCustomValidationConcurrency, + maxFetchConcurrency, } = _workerData as ValidateDocumentsWorkerData if (isMainThread || !parentPort) { @@ -359,6 +361,7 @@ async function validateDocuments() { getDocumentExists, environment: 'cli', maxCustomValidationConcurrency, + maxFetchConcurrency, }), new Promise((resolve) => setTimeout(() => resolve(timeout), DOCUMENT_VALIDATION_TIMEOUT), diff --git a/packages/sanity/src/core/validation/validateDocument.ts b/packages/sanity/src/core/validation/validateDocument.ts index 03c482fdbf32..dfa9908d3a5f 100644 --- a/packages/sanity/src/core/validation/validateDocument.ts +++ b/packages/sanity/src/core/validation/validateDocument.ts @@ -24,10 +24,23 @@ import {typeString} from './util/typeString' import {unknownFieldsValidator} from './validators/unknownFieldsValidator' // this is the number of requests allowed inflight at once. this is done to prevent -// the validation library from overwhelming our backend -const MAX_FETCH_CONCURRENCY = 10 - -const limitConcurrency = createClientConcurrencyLimiter(MAX_FETCH_CONCURRENCY) +// the validation library from overwhelming our backend. +// NOTE: this was upped from 10 to prevent issues where many concurrency +// `client.fetch` requests would "clog" custom validators from finishing due to +// not enough concurrent requests being fulfilled +// +// NOTE: ensure to update the JSDoc below if this number is changed +const DEFAULT_MAX_FETCH_CONCURRENCY = 25 + +// NOTE: update the TSDoc in `ValidateDocumentOptions` if this is changed. +const DEFAULT_MAX_CUSTOM_VALIDATION_CONCURRENCY = 5 + +let _limitConcurrency: ReturnType | undefined +const getConcurrencyLimiter = (maxConcurrency: number) => { + if (_limitConcurrency) return _limitConcurrency + _limitConcurrency = createClientConcurrencyLimiter(maxConcurrency) + return _limitConcurrency +} const isRecord = (maybeRecord: unknown): maybeRecord is Record => typeof maybeRecord === 'object' && maybeRecord !== null && !Array.isArray(maybeRecord) @@ -104,8 +117,21 @@ export interface ValidateDocumentOptions { * concurrently at once. This helps prevent custom validators from * overwhelming backend services (e.g. called via fetch) used in async, * user-defined validation functions. (i.e. `rule.custom(async() => {})`) + * + * Note that lowering this number may also help in cases where a custom + * validator could potentially exhaust the fetch concurrency. This is 5 by + * default. */ maxCustomValidationConcurrency?: number + + /** + * The amount of allowed inflight fetch requests at once. You may need to up + * this value if you have complex custom validations that require many + * `client.fetch` requests at once. It's possible for custom validator to + * stall if there are not enough concurrent fetch requests available to + * fullfil the custom validation. This is 25 by default. + */ + maxFetchConcurrency?: number } /** @@ -118,9 +144,13 @@ export function validateDocument({ document, workspace, environment = 'studio', + maxFetchConcurrency, ...options }: ValidateDocumentOptions): Promise { const getClient = options.getClient || workspace.getClient + const limitConcurrency = getConcurrencyLimiter( + maxFetchConcurrency ?? DEFAULT_MAX_FETCH_CONCURRENCY, + ) const getConcurrencyLimitedClient = (clientOptions: SourceClientOptions) => limitConcurrency(getClient(clientOptions)) @@ -190,8 +220,10 @@ export function validateDocumentObservable({ } let customValidationConcurrencyLimiter = customValidationConcurrencyLimiters.get(schema) - if (!customValidationConcurrencyLimiter && maxCustomValidationConcurrency) { - customValidationConcurrencyLimiter = new ConcurrencyLimiter(maxCustomValidationConcurrency) + if (!customValidationConcurrencyLimiter) { + customValidationConcurrencyLimiter = new ConcurrencyLimiter( + maxCustomValidationConcurrency ?? DEFAULT_MAX_CUSTOM_VALIDATION_CONCURRENCY, + ) customValidationConcurrencyLimiters.set(schema, customValidationConcurrencyLimiter) }