From 6f2e5f2f61a6cb6bf2184e84e13759af441e00e4 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Thu, 3 Apr 2025 16:55:58 -0400 Subject: [PATCH 01/11] remote_store.ts: let mutations accrue for 250ms and send them in batches. --- packages/firestore/src/remote/remote_store.ts | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index 6f8aed0503e..e5034721c11 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -126,6 +126,10 @@ class RemoteStoreImpl implements RemoteStore { */ writePipeline: MutationBatch[] = []; + sentWrites = new WeakSet(); + + writeTimeoutId: ReturnType | null = null; + /** * A mapping of watched targets that the client cares about tracking and the * user has explicitly called a 'listen' for this target. @@ -734,10 +738,23 @@ function addToWritePipeline( ); remoteStoreImpl.writePipeline.push(batch); - const writeStream = ensureWriteStream(remoteStoreImpl); - if (writeStream.isOpen() && writeStream.handshakeComplete) { - writeStream.writeMutations(batch.mutations); + if (remoteStoreImpl.writeTimeoutId !== null) { + return; } + + remoteStoreImpl.writeTimeoutId = setTimeout(() => { + remoteStoreImpl.writeTimeoutId = null; + const writeStream = ensureWriteStream(remoteStoreImpl); + if (writeStream.isOpen() && writeStream.handshakeComplete) { + for (const curBatch of remoteStoreImpl.writePipeline) { + if (remoteStoreImpl.sentWrites.has(curBatch)) { + continue; + } + writeStream.writeMutations(curBatch.mutations); + remoteStoreImpl.sentWrites.add(curBatch); + } + } + }, 200); } function shouldStartWriteStream(remoteStoreImpl: RemoteStoreImpl): boolean { @@ -769,6 +786,7 @@ async function onWriteHandshakeComplete( // Send the write pipeline now that the stream is established. for (const batch of remoteStoreImpl.writePipeline) { writeStream.writeMutations(batch.mutations); + remoteStoreImpl.sentWrites.add(batch); } } From 06cc13fc65db42890035ef6593a45536739419ad Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 7 Apr 2025 16:38:06 +0000 Subject: [PATCH 02/11] use AsyncQueue instead of calling setTimeout directly --- packages/firestore/src/remote/remote_store.ts | 76 ++++++++++++------- packages/firestore/src/util/async_queue.ts | 1 + 2 files changed, 51 insertions(+), 26 deletions(-) diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index e5034721c11..4833897fa45 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -28,7 +28,7 @@ import { TargetData } from '../local/target_data'; import { MutationResult } from '../model/mutation'; import { MutationBatch, MutationBatchResult } from '../model/mutation_batch'; import { debugAssert, debugCast } from '../util/assert'; -import { AsyncQueue } from '../util/async_queue'; +import { AsyncQueue, DelayedOperation, TimerId } from '../util/async_queue'; import { ByteString } from '../util/byte_string'; import { FirestoreError } from '../util/error'; import { logDebug } from '../util/log'; @@ -77,6 +77,11 @@ const enum OfflineCause { Shutdown } +export interface WritePipelineEntry { + mutationBatch: MutationBatch; + sent: boolean; +} + /** * RemoteStore - An interface to remotely stored data, basically providing a * wrapper around the Datastore that is more reliable for the rest of the @@ -124,11 +129,15 @@ class RemoteStoreImpl implements RemoteStore { * purely based on order, and so we can just shift() writes from the front of * the writePipeline as we receive responses. */ - writePipeline: MutationBatch[] = []; - - sentWrites = new WeakSet(); + writePipeline: WritePipelineEntry[] = []; - writeTimeoutId: ReturnType | null = null; + /** + * The operation that is enqueued to send writes from the `writePipeline` that + * have not yet been sent. The delay is used to enable batching of writes, + * reducing the overall number of HTTP requests that need to be made to the + * backend. + */ + sendWritesOperation: DelayedOperation | null = null; /** * A mapping of watched targets that the client cares about tracking and the @@ -678,7 +687,7 @@ export async function fillWritePipeline( let lastBatchIdRetrieved = remoteStoreImpl.writePipeline.length > 0 ? remoteStoreImpl.writePipeline[remoteStoreImpl.writePipeline.length - 1] - .batchId + .mutationBatch.batchId : BATCHID_UNKNOWN; while (canAddToWritePipeline(remoteStoreImpl)) { @@ -736,25 +745,33 @@ function addToWritePipeline( canAddToWritePipeline(remoteStoreImpl), 'addToWritePipeline called when pipeline is full' ); - remoteStoreImpl.writePipeline.push(batch); + remoteStoreImpl.writePipeline.push({ + mutationBatch: batch, + sent: false + }); - if (remoteStoreImpl.writeTimeoutId !== null) { + if (remoteStoreImpl.sendWritesOperation !== null) { return; } - remoteStoreImpl.writeTimeoutId = setTimeout(() => { - remoteStoreImpl.writeTimeoutId = null; - const writeStream = ensureWriteStream(remoteStoreImpl); - if (writeStream.isOpen() && writeStream.handshakeComplete) { - for (const curBatch of remoteStoreImpl.writePipeline) { - if (remoteStoreImpl.sentWrites.has(curBatch)) { - continue; + remoteStoreImpl.sendWritesOperation = + remoteStoreImpl.asyncQueue.enqueueAfterDelay( + TimerId.WriteStreamSendDelay, + 200, + () => { + remoteStoreImpl.sendWritesOperation = null; + const writeStream = ensureWriteStream(remoteStoreImpl); + if (writeStream.isOpen() && writeStream.handshakeComplete) { + for (const pipelineEntry of remoteStoreImpl.writePipeline) { + if (!pipelineEntry.sent) { + writeStream.writeMutations(pipelineEntry.mutationBatch.mutations); + pipelineEntry.sent = true; + } + } } - writeStream.writeMutations(curBatch.mutations); - remoteStoreImpl.sentWrites.add(curBatch); + return Promise.resolve(); } - } - }, 200); + ); } function shouldStartWriteStream(remoteStoreImpl: RemoteStoreImpl): boolean { @@ -784,9 +801,9 @@ async function onWriteHandshakeComplete( ): Promise { const writeStream = ensureWriteStream(remoteStoreImpl); // Send the write pipeline now that the stream is established. - for (const batch of remoteStoreImpl.writePipeline) { - writeStream.writeMutations(batch.mutations); - remoteStoreImpl.sentWrites.add(batch); + for (const pipelineEntry of remoteStoreImpl.writePipeline) { + writeStream.writeMutations(pipelineEntry.mutationBatch.mutations); + pipelineEntry.sent = true; } } @@ -801,8 +818,12 @@ async function onMutationResult( remoteStoreImpl.writePipeline.length > 0, 'Got result for empty write pipeline' ); - const batch = remoteStoreImpl.writePipeline.shift()!; - const success = MutationBatchResult.from(batch, commitVersion, results); + const pipelineEntry = remoteStoreImpl.writePipeline.shift()!; + const success = MutationBatchResult.from( + pipelineEntry.mutationBatch, + commitVersion, + results + ); debugAssert( !!remoteStoreImpl.remoteSyncer.applySuccessfulWrite, @@ -853,7 +874,7 @@ async function handleWriteError( if (isPermanentWriteError(error.code)) { // This was a permanent error, the request itself was the problem // so it's not going to succeed if we resend it. - const batch = remoteStoreImpl.writePipeline.shift()!; + const pipelineEntry = remoteStoreImpl.writePipeline.shift()!; // In this case it's also unlikely that the server itself is melting // down -- this was just a bad request so inhibit backoff on the next @@ -865,7 +886,10 @@ async function handleWriteError( 'rejectFailedWrite() not set' ); await executeWithRecovery(remoteStoreImpl, () => - remoteStoreImpl.remoteSyncer.rejectFailedWrite!(batch.batchId, error) + remoteStoreImpl.remoteSyncer.rejectFailedWrite!( + pipelineEntry.mutationBatch.batchId, + error + ) ); // It's possible that with the completion of this mutation diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts index 09171a9f038..6aed65d670d 100644 --- a/packages/firestore/src/util/async_queue.ts +++ b/packages/firestore/src/util/async_queue.ts @@ -48,6 +48,7 @@ export const enum TimerId { ListenStreamConnectionBackoff = 'listen_stream_connection_backoff', WriteStreamIdle = 'write_stream_idle', WriteStreamConnectionBackoff = 'write_stream_connection_backoff', + WriteStreamSendDelay = 'write_stream_send_delay', HealthCheckTimeout = 'health_check_timeout', /** From 6ad53b6aec792b376b397904976d808e1071bc3d Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 7 Apr 2025 17:23:46 +0000 Subject: [PATCH 03/11] remote_store.ts: rename the concept of "sendWritesOperation" to "sendWriteRequestsOperation" --- packages/firestore/src/remote/remote_store.ts | 88 ++++++++++++------- packages/firestore/src/util/async_queue.ts | 11 ++- 2 files changed, 67 insertions(+), 32 deletions(-) diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index 4833897fa45..9642e03335b 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -79,7 +79,7 @@ const enum OfflineCause { export interface WritePipelineEntry { mutationBatch: MutationBatch; - sent: boolean; + writeRequestSent: boolean; } /** @@ -132,12 +132,16 @@ class RemoteStoreImpl implements RemoteStore { writePipeline: WritePipelineEntry[] = []; /** - * The operation that is enqueued to send writes from the `writePipeline` that - * have not yet been sent. The delay is used to enable batching of writes, - * reducing the overall number of HTTP requests that need to be made to the - * backend. + * The operation that is enqueued to send unsent write requests enqueued in + * the write pipeline. + * + * If the `sendWriteRequestsDelayMs` argument specified to the constructor is + * `null` then this property will _never_ be set to a non-null value; + * otherwise, it will be set to a non-null value when such an operation is + * enqueued in the AsyncQueue, and will be set back to `null` once that + * operation has completed or been cancelled. */ - sendWritesOperation: DelayedOperation | null = null; + sendWriteRequestsOperation: DelayedOperation | null = null; /** * A mapping of watched targets that the client cares about tracking and the @@ -181,8 +185,21 @@ class RemoteStoreImpl implements RemoteStore { readonly datastore: Datastore, readonly asyncQueue: AsyncQueue, onlineStateHandler: (onlineState: OnlineState) => void, - connectivityMonitor: ConnectivityMonitor + connectivityMonitor: ConnectivityMonitor, + readonly sendWriteRequestsDelayMs: number | null ) { + if (sendWriteRequestsDelayMs !== null) { + debugAssert( + Number.isInteger(sendWriteRequestsDelayMs), + `invalid sendWriteRequestsDelayMs: ${sendWriteRequestsDelayMs} ` + + '(must be an integer, not a fractional amount)' + ); + debugAssert( + sendWriteRequestsDelayMs > 0, + `invalid sendWriteRequestsDelayMs: ${sendWriteRequestsDelayMs} ` + + '(must be greater than zero)' + ); + } this.connectivityMonitor = connectivityMonitor; this.connectivityMonitor.addCallback((_: NetworkStatus) => { asyncQueue.enqueueAndForget(async () => { @@ -211,14 +228,16 @@ export function newRemoteStore( datastore: Datastore, asyncQueue: AsyncQueue, onlineStateHandler: (onlineState: OnlineState) => void, - connectivityMonitor: ConnectivityMonitor + connectivityMonitor: ConnectivityMonitor, + sendWriteRequestsDelayMs: number | null ): RemoteStore { return new RemoteStoreImpl( localStore, datastore, asyncQueue, onlineStateHandler, - connectivityMonitor + connectivityMonitor, + sendWriteRequestsDelayMs ); } @@ -747,31 +766,40 @@ function addToWritePipeline( ); remoteStoreImpl.writePipeline.push({ mutationBatch: batch, - sent: false + writeRequestSent: false }); - if (remoteStoreImpl.sendWritesOperation !== null) { - return; + if ( + remoteStoreImpl.sendWriteRequestsDelayMs === null || + !canAddToWritePipeline(remoteStoreImpl) + ) { + remoteStoreImpl.sendWriteRequestsOperation?.cancel(); + remoteStoreImpl.sendWriteRequestsOperation = null; + sendWriteRequestsFromPipeline(remoteStoreImpl); + } else if (remoteStoreImpl.sendWriteRequestsOperation === null) { + remoteStoreImpl.sendWriteRequestsOperation = + remoteStoreImpl.asyncQueue.enqueueAfterDelay( + TimerId.RemoteStoreSendWriteRequests, + remoteStoreImpl.sendWriteRequestsDelayMs, + () => { + remoteStoreImpl.sendWriteRequestsOperation = null; + sendWriteRequestsFromPipeline(remoteStoreImpl); + return Promise.resolve(); + } + ); } +} - remoteStoreImpl.sendWritesOperation = - remoteStoreImpl.asyncQueue.enqueueAfterDelay( - TimerId.WriteStreamSendDelay, - 200, - () => { - remoteStoreImpl.sendWritesOperation = null; - const writeStream = ensureWriteStream(remoteStoreImpl); - if (writeStream.isOpen() && writeStream.handshakeComplete) { - for (const pipelineEntry of remoteStoreImpl.writePipeline) { - if (!pipelineEntry.sent) { - writeStream.writeMutations(pipelineEntry.mutationBatch.mutations); - pipelineEntry.sent = true; - } - } - } - return Promise.resolve(); +function sendWriteRequestsFromPipeline(remoteStoreImpl: RemoteStoreImpl): void { + const writeStream = ensureWriteStream(remoteStoreImpl); + if (writeStream.isOpen() && writeStream.handshakeComplete) { + for (const pipelineEntry of remoteStoreImpl.writePipeline) { + if (!pipelineEntry.writeRequestSent) { + writeStream.writeMutations(pipelineEntry.mutationBatch.mutations); + pipelineEntry.writeRequestSent = true; } - ); + } + } } function shouldStartWriteStream(remoteStoreImpl: RemoteStoreImpl): boolean { @@ -803,7 +831,7 @@ async function onWriteHandshakeComplete( // Send the write pipeline now that the stream is established. for (const pipelineEntry of remoteStoreImpl.writePipeline) { writeStream.writeMutations(pipelineEntry.mutationBatch.mutations); - pipelineEntry.sent = true; + pipelineEntry.writeRequestSent = true; } } diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts index 6aed65d670d..c2faccfc31a 100644 --- a/packages/firestore/src/util/async_queue.ts +++ b/packages/firestore/src/util/async_queue.ts @@ -48,7 +48,6 @@ export const enum TimerId { ListenStreamConnectionBackoff = 'listen_stream_connection_backoff', WriteStreamIdle = 'write_stream_idle', WriteStreamConnectionBackoff = 'write_stream_connection_backoff', - WriteStreamSendDelay = 'write_stream_send_delay', HealthCheckTimeout = 'health_check_timeout', /** @@ -82,7 +81,15 @@ export const enum TimerId { /** * A timer used to periodically attempt index backfill. */ - IndexBackfill = 'index_backfill' + IndexBackfill = 'index_backfill', + + /** + * A timer used to send write requests for mutations to the write stream in + * the remote store. The timer is used when the remote store is configured to + * have a small delay before sending write requests to the backend so that + * multiple writes can be batched into a single HTTP request. + */ + RemoteStoreSendWriteRequests = 'remote_store_send_write_requests' } /** From fd77d62c7afbb9a89669eceef58797b1094d5751 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 7 Apr 2025 18:27:55 +0000 Subject: [PATCH 04/11] ExperimentalOptions added --- packages/firestore/src/api/database.ts | 1 + .../firestore/src/api/experimental_options.ts | 76 +++++++++++++++++++ .../firestore/src/core/component_provider.ts | 4 +- .../firestore/src/core/firestore_client.ts | 4 +- packages/firestore/src/lite-api/settings.ts | 43 ++++++++++- .../test/unit/specs/spec_test_runner.ts | 13 +++- 6 files changed, 136 insertions(+), 5 deletions(-) create mode 100644 packages/firestore/src/api/experimental_options.ts diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index 0757378a74c..f2756a20e4e 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -310,6 +310,7 @@ export function configureFirestore(firestore: Firestore): void { firestore._appCheckCredentials, firestore._queue, databaseInfo, + settings.experimentalOptions.sendWriteRequestsDelayMs ?? null, firestore._componentsProvider && buildComponentProvider(firestore._componentsProvider) ); diff --git a/packages/firestore/src/api/experimental_options.ts b/packages/firestore/src/api/experimental_options.ts new file mode 100644 index 00000000000..3fe711eb870 --- /dev/null +++ b/packages/firestore/src/api/experimental_options.ts @@ -0,0 +1,76 @@ +/** + * @license + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Experimental options to configure the Firestore SDK. + * + * Note: This interface is "experimental" and is subject to change. + * + * See `FirestoreSettings.experimental`. + */ +export interface ExperimentalOptions { + /** + * The maximum amount of time, in milliseconds, to wait before sending a + * Firestore "write" request to the backend. If `undefined` then do not delay + * at all. + * + * A delay can be useful because it enables the in-memory "write pipeline" to + * gather together multiple write requests and send them in a single HTTP + * request to the backend, rather than one HTTP request per write request, as + * is done when by default, or when this property is `undefined`. Note that + * there is a hardcoded limit to the number of write requests that are sent at + * once, so setting a very large value for this property will not necessarily + * cause _all_ write requests to be sent in a single HTTP request; however, it + * _could_ greatly reduce the number of distinct HTTP requests that are used. + * + * The value must be an integer value strictly greater than zero and less than + * or equal to 2000 (2 seconds). A value of `200` is a good starting point to + * minimize write latency yet still enable a some amount of batching. + * + * See https://github.com/firebase/firebase-js-sdk/issues/5971 for rationale + * and background information that motivated this option. + */ + sendWriteRequestsDelayMs?: number; +} + +/** + * Compares two `ExperimentalOptions` objects for equality. + */ +export function experimentalOptionsEqual( + options1: ExperimentalOptions, + options2: ExperimentalOptions +): boolean { + return ( + options1.sendWriteRequestsDelayMs === options2.sendWriteRequestsDelayMs + ); +} + +/** + * Creates and returns a new `ExperimentalOptions` with the same + * option values as the given instance. + */ +export function cloneExperimentalOptions( + options: ExperimentalOptions +): ExperimentalOptions { + const clone: ExperimentalOptions = {}; + + if (options.sendWriteRequestsDelayMs !== undefined) { + clone.sendWriteRequestsDelayMs = options.sendWriteRequestsDelayMs; + } + + return clone; +} diff --git a/packages/firestore/src/core/component_provider.ts b/packages/firestore/src/core/component_provider.ts index 8a63509232c..9f6c8ec0cdc 100644 --- a/packages/firestore/src/core/component_provider.ts +++ b/packages/firestore/src/core/component_provider.ts @@ -85,6 +85,7 @@ export interface ComponentConfiguration { clientId: ClientId; initialUser: User; maxConcurrentLimboResolutions: number; + sendWriteRequestsDelayMs: number | null; } export interface OfflineComponentProviderFactory { @@ -485,7 +486,8 @@ export class OnlineComponentProvider { onlineState, OnlineStateSource.RemoteStore ), - newConnectivityMonitor() + newConnectivityMonitor(), + cfg.sendWriteRequestsDelayMs ); } diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index e2aa19aaba8..efe5426a301 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -140,6 +140,7 @@ export class FirestoreClient { */ public asyncQueue: AsyncQueue, private databaseInfo: DatabaseInfo, + private sendWriteRequestsDelayMs: number | null, componentProvider?: { _offline: OfflineComponentProvider; _online: OnlineComponentProvider; @@ -165,7 +166,8 @@ export class FirestoreClient { authCredentials: this.authCredentials, appCheckCredentials: this.appCheckCredentials, initialUser: this.user, - maxConcurrentLimboResolutions: MAX_CONCURRENT_LIMBO_RESOLUTIONS + maxConcurrentLimboResolutions: MAX_CONCURRENT_LIMBO_RESOLUTIONS, + sendWriteRequestsDelayMs: this.sendWriteRequestsDelayMs }; } diff --git a/packages/firestore/src/lite-api/settings.ts b/packages/firestore/src/lite-api/settings.ts index a1bba373d13..914d8068468 100644 --- a/packages/firestore/src/lite-api/settings.ts +++ b/packages/firestore/src/lite-api/settings.ts @@ -24,6 +24,11 @@ import { cloneLongPollingOptions, longPollingOptionsEqual } from '../api/long_polling_options'; +import { + ExperimentalOptions, + cloneExperimentalOptions, + experimentalOptionsEqual +} from '../api/experimental_options'; import { LRU_COLLECTION_DISABLED, LRU_DEFAULT_CACHE_SIZE_BYTES @@ -50,6 +55,8 @@ const MAX_LONG_POLLING_TIMEOUT_SECONDS = 30; // Whether long-polling auto-detected is enabled by default. const DEFAULT_AUTO_DETECT_LONG_POLLING = true; +const MAX_SEND_WRITE_REQUEST_DELAY_MS = 2000; + /** * Specifies custom configurations for your Cloud Firestore instance. * You must set these before invoking any other methods. @@ -83,6 +90,7 @@ export interface PrivateSettings extends FirestoreSettings { experimentalLongPollingOptions?: ExperimentalLongPollingOptions; useFetchStreams?: boolean; emulatorOptions?: { mockUserToken?: EmulatorMockTokenOptions | string }; + experimentalOptions?: ExperimentalOptions; localCache?: FirestoreLocalCache; } @@ -111,6 +119,7 @@ export class FirestoreSettingsImpl { readonly useFetchStreams: boolean; readonly localCache?: FirestoreLocalCache; + readonly experimentalOptions: ExperimentalOptions; // Can be a google-auth-library or gapi client. // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -178,6 +187,11 @@ export class FirestoreSettingsImpl { validateLongPollingOptions(this.experimentalLongPollingOptions); this.useFetchStreams = !!settings.useFetchStreams; + + this.experimentalOptions = cloneExperimentalOptions( + settings.experimentalOptions ?? {} + ); + validateExperimentalOptions(this.experimentalOptions); } isEqual(other: FirestoreSettingsImpl): boolean { @@ -195,7 +209,11 @@ export class FirestoreSettingsImpl { other.experimentalLongPollingOptions ) && this.ignoreUndefinedProperties === other.ignoreUndefinedProperties && - this.useFetchStreams === other.useFetchStreams + this.useFetchStreams === other.useFetchStreams && + experimentalOptionsEqual( + this.experimentalOptions, + other.experimentalOptions + ) ); } } @@ -227,3 +245,26 @@ function validateLongPollingOptions( } } } + +function validateExperimentalOptions(options: ExperimentalOptions): void { + if (options.sendWriteRequestsDelayMs !== undefined) { + if (isNaN(options.sendWriteRequestsDelayMs)) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `invalid sendWriteRequestsDelayMs: ` + + `${options.sendWriteRequestsDelayMs} (must not be NaN)` + ); + } + if ( + options.sendWriteRequestsDelayMs <= 0 || + options.sendWriteRequestsDelayMs > MAX_SEND_WRITE_REQUEST_DELAY_MS + ) { + throw new FirestoreError( + Code.INVALID_ARGUMENT, + `invalid sendWriteRequestsDelayMs: ` + + `${options.sendWriteRequestsDelayMs} (must be greater than zero ` + + `and less than or equal to ${MAX_SEND_WRITE_REQUEST_DELAY_MS})` + ); + } + } +} diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index b34421d9e0a..b4b9dceb101 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -259,6 +259,7 @@ abstract class TestRunner { private useEagerGCForMemory: boolean; private numClients: number; private maxConcurrentLimboResolutions?: number; + private sendWriteRequestsDelayMs?: number; private databaseInfo: DatabaseInfo; protected user = User.UNAUTHENTICATED; @@ -299,6 +300,7 @@ abstract class TestRunner { this.useEagerGCForMemory = config.useEagerGCForMemory; this.numClients = config.numClients; this.maxConcurrentLimboResolutions = config.maxConcurrentLimboResolutions; + this.sendWriteRequestsDelayMs = config.sendWriteRequestsDelayMs; this.expectedActiveLimboDocs = []; this.expectedEnqueuedLimboDocs = []; this.expectedActiveTargets = new Map(); @@ -316,8 +318,9 @@ abstract class TestRunner { clientId: this.clientId, initialUser: this.user, maxConcurrentLimboResolutions: - this.maxConcurrentLimboResolutions ?? Number.MAX_SAFE_INTEGER - }; + this.maxConcurrentLimboResolutions ?? Number.MAX_SAFE_INTEGER, + sendWriteRequestsDelayMs: this.sendWriteRequestsDelayMs ?? null + } satisfies ComponentConfiguration; this.connection = new MockConnection(this.queue); @@ -1408,6 +1411,12 @@ export interface SpecConfig { * default value. */ maxConcurrentLimboResolutions?: number; + + /** + * The maximum number amount of time, in milliseconds, to delay sending + * write requests to the backend in the remote store. + */ + sendWriteRequestsDelayMs?: number; } /** From 67a5a3514dd0153d2b119230969c55b00b2636a9 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 7 Apr 2025 18:49:16 +0000 Subject: [PATCH 05/11] work --- packages/firestore/src/api/database.ts | 1 - packages/firestore/src/core/component_provider.ts | 3 +-- packages/firestore/src/core/database_info.ts | 5 ++++- packages/firestore/src/core/firestore_client.ts | 4 +--- packages/firestore/src/lite-api/components.ts | 3 ++- packages/firestore/src/lite-api/settings.ts | 15 +++++---------- .../test/integration/util/internal_helpers.ts | 3 ++- .../test/unit/remote/rest_connection.test.ts | 3 ++- .../firestore/test/unit/specs/spec_test_runner.ts | 7 +++---- 9 files changed, 20 insertions(+), 24 deletions(-) diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts index f2756a20e4e..0757378a74c 100644 --- a/packages/firestore/src/api/database.ts +++ b/packages/firestore/src/api/database.ts @@ -310,7 +310,6 @@ export function configureFirestore(firestore: Firestore): void { firestore._appCheckCredentials, firestore._queue, databaseInfo, - settings.experimentalOptions.sendWriteRequestsDelayMs ?? null, firestore._componentsProvider && buildComponentProvider(firestore._componentsProvider) ); diff --git a/packages/firestore/src/core/component_provider.ts b/packages/firestore/src/core/component_provider.ts index 9f6c8ec0cdc..9f59502285c 100644 --- a/packages/firestore/src/core/component_provider.ts +++ b/packages/firestore/src/core/component_provider.ts @@ -85,7 +85,6 @@ export interface ComponentConfiguration { clientId: ClientId; initialUser: User; maxConcurrentLimboResolutions: number; - sendWriteRequestsDelayMs: number | null; } export interface OfflineComponentProviderFactory { @@ -487,7 +486,7 @@ export class OnlineComponentProvider { OnlineStateSource.RemoteStore ), newConnectivityMonitor(), - cfg.sendWriteRequestsDelayMs + cfg.databaseInfo.sendWriteRequestsDelayMs ); } diff --git a/packages/firestore/src/core/database_info.ts b/packages/firestore/src/core/database_info.ts index 0325f8166b6..c8fc6c82247 100644 --- a/packages/firestore/src/core/database_info.ts +++ b/packages/firestore/src/core/database_info.ts @@ -38,6 +38,8 @@ export class DatabaseInfo { * @param longPollingOptions Options that configure long-polling. * @param useFetchStreams Whether to use the Fetch API instead of * XMLHTTPRequest + * @param sendWriteRequestsDelayMs The delay, in milliseconds, to use before + * sending write requests over the wire in remote store. */ constructor( readonly databaseId: DatabaseId, @@ -48,7 +50,8 @@ export class DatabaseInfo { readonly forceLongPolling: boolean, readonly autoDetectLongPolling: boolean, readonly longPollingOptions: ExperimentalLongPollingOptions, - readonly useFetchStreams: boolean + readonly useFetchStreams: boolean, + readonly sendWriteRequestsDelayMs: number | null ) {} } diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index efe5426a301..e2aa19aaba8 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -140,7 +140,6 @@ export class FirestoreClient { */ public asyncQueue: AsyncQueue, private databaseInfo: DatabaseInfo, - private sendWriteRequestsDelayMs: number | null, componentProvider?: { _offline: OfflineComponentProvider; _online: OnlineComponentProvider; @@ -166,8 +165,7 @@ export class FirestoreClient { authCredentials: this.authCredentials, appCheckCredentials: this.appCheckCredentials, initialUser: this.user, - maxConcurrentLimboResolutions: MAX_CONCURRENT_LIMBO_RESOLUTIONS, - sendWriteRequestsDelayMs: this.sendWriteRequestsDelayMs + maxConcurrentLimboResolutions: MAX_CONCURRENT_LIMBO_RESOLUTIONS }; } diff --git a/packages/firestore/src/lite-api/components.ts b/packages/firestore/src/lite-api/components.ts index 436d2b5d4d8..23764f58857 100644 --- a/packages/firestore/src/lite-api/components.ts +++ b/packages/firestore/src/lite-api/components.ts @@ -119,6 +119,7 @@ export function makeDatabaseInfo( settings.experimentalForceLongPolling, settings.experimentalAutoDetectLongPolling, cloneLongPollingOptions(settings.experimentalLongPollingOptions), - settings.useFetchStreams + settings.useFetchStreams, + settings.experimental.sendWriteRequestsDelayMs ?? null ); } diff --git a/packages/firestore/src/lite-api/settings.ts b/packages/firestore/src/lite-api/settings.ts index 914d8068468..2bdc3ebc2f9 100644 --- a/packages/firestore/src/lite-api/settings.ts +++ b/packages/firestore/src/lite-api/settings.ts @@ -90,7 +90,7 @@ export interface PrivateSettings extends FirestoreSettings { experimentalLongPollingOptions?: ExperimentalLongPollingOptions; useFetchStreams?: boolean; emulatorOptions?: { mockUserToken?: EmulatorMockTokenOptions | string }; - experimentalOptions?: ExperimentalOptions; + experimental?: ExperimentalOptions; localCache?: FirestoreLocalCache; } @@ -119,7 +119,7 @@ export class FirestoreSettingsImpl { readonly useFetchStreams: boolean; readonly localCache?: FirestoreLocalCache; - readonly experimentalOptions: ExperimentalOptions; + readonly experimental: ExperimentalOptions; // Can be a google-auth-library or gapi client. // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -188,10 +188,8 @@ export class FirestoreSettingsImpl { this.useFetchStreams = !!settings.useFetchStreams; - this.experimentalOptions = cloneExperimentalOptions( - settings.experimentalOptions ?? {} - ); - validateExperimentalOptions(this.experimentalOptions); + this.experimental = cloneExperimentalOptions(settings.experimental ?? {}); + validateExperimentalOptions(this.experimental); } isEqual(other: FirestoreSettingsImpl): boolean { @@ -210,10 +208,7 @@ export class FirestoreSettingsImpl { ) && this.ignoreUndefinedProperties === other.ignoreUndefinedProperties && this.useFetchStreams === other.useFetchStreams && - experimentalOptionsEqual( - this.experimentalOptions, - other.experimentalOptions - ) + experimentalOptionsEqual(this.experimental, other.experimental) ); } } diff --git a/packages/firestore/test/integration/util/internal_helpers.ts b/packages/firestore/test/integration/util/internal_helpers.ts index 86ded6af3c1..558796ce836 100644 --- a/packages/firestore/test/integration/util/internal_helpers.ts +++ b/packages/firestore/test/integration/util/internal_helpers.ts @@ -61,7 +61,8 @@ export function getDefaultDatabaseInfo(): DatabaseInfo { cloneLongPollingOptions( DEFAULT_SETTINGS.experimentalLongPollingOptions ?? {} ), - /*use FetchStreams= */ false + /*use FetchStreams= */ false, + /*sendWriteRequestsDelayMs= */ null ); } diff --git a/packages/firestore/test/unit/remote/rest_connection.test.ts b/packages/firestore/test/unit/remote/rest_connection.test.ts index d45a75ce67b..216b784ecd2 100644 --- a/packages/firestore/test/unit/remote/rest_connection.test.ts +++ b/packages/firestore/test/unit/remote/rest_connection.test.ts @@ -67,7 +67,8 @@ describe('RestConnection', () => { /*forceLongPolling=*/ false, /*autoDetectLongPolling=*/ false, /*longPollingOptions=*/ {}, - /*useFetchStreams=*/ false + /*useFetchStreams=*/ false, + /*sendWriteRequestsDelayMs= */ null ); const connection = new TestRestConnection(testDatabaseInfo); diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index b4b9dceb101..14dd73e1e2f 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -283,7 +283,8 @@ abstract class TestRunner { /*forceLongPolling=*/ false, /*autoDetectLongPolling=*/ false, /*longPollingOptions=*/ {}, - /*useFetchStreams=*/ false + /*useFetchStreams=*/ false, + config.sendWriteRequestsDelayMs ?? null ); // TODO(mrschmidt): During client startup in `firestore_client`, we block @@ -300,7 +301,6 @@ abstract class TestRunner { this.useEagerGCForMemory = config.useEagerGCForMemory; this.numClients = config.numClients; this.maxConcurrentLimboResolutions = config.maxConcurrentLimboResolutions; - this.sendWriteRequestsDelayMs = config.sendWriteRequestsDelayMs; this.expectedActiveLimboDocs = []; this.expectedEnqueuedLimboDocs = []; this.expectedActiveTargets = new Map(); @@ -318,8 +318,7 @@ abstract class TestRunner { clientId: this.clientId, initialUser: this.user, maxConcurrentLimboResolutions: - this.maxConcurrentLimboResolutions ?? Number.MAX_SAFE_INTEGER, - sendWriteRequestsDelayMs: this.sendWriteRequestsDelayMs ?? null + this.maxConcurrentLimboResolutions ?? Number.MAX_SAFE_INTEGER } satisfies ComponentConfiguration; this.connection = new MockConnection(this.queue); From 4c5cae9c0c2e3436fcbcbbd1c2b98ae89cba425f Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 7 Apr 2025 19:18:17 +0000 Subject: [PATCH 06/11] fix --- packages/firestore/src/api.ts | 1 + packages/firestore/src/api/settings.ts | 7 +++++++ packages/firestore/src/lite-api/settings.ts | 6 ++++++ packages/firestore/src/register.ts | 1 + packages/firestore/src/remote/remote_store.ts | 10 +--------- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index ea969c6b94c..1cdf6980eda 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -84,6 +84,7 @@ export { export { FirestoreSettings, PersistenceSettings } from './api/settings'; export type { PrivateSettings } from './lite-api/settings'; export { ExperimentalLongPollingOptions } from './api/long_polling_options'; +export { ExperimentalOptions } from './api/experimental_options'; export { DocumentChange, diff --git a/packages/firestore/src/api/settings.ts b/packages/firestore/src/api/settings.ts index b47017bbc2e..c5381608abf 100644 --- a/packages/firestore/src/api/settings.ts +++ b/packages/firestore/src/api/settings.ts @@ -19,6 +19,7 @@ import { FirestoreSettings as LiteSettings } from '../lite-api/settings'; import { FirestoreLocalCache } from './cache_config'; import { ExperimentalLongPollingOptions } from './long_polling_options'; +import { ExperimentalOptions } from './experimental_options'; export { DEFAULT_HOST } from '../lite-api/settings'; @@ -114,4 +115,10 @@ export interface FirestoreSettings extends LiteSettings { * effect. */ experimentalLongPollingOptions?: ExperimentalLongPollingOptions; + + /** + * Options that are "experimental", meaning that their semantics are subject + * to change at any time without notice, up to and including complete removal. + */ + experimental?: ExperimentalOptions; } diff --git a/packages/firestore/src/lite-api/settings.ts b/packages/firestore/src/lite-api/settings.ts index 2bdc3ebc2f9..b3a50b86b72 100644 --- a/packages/firestore/src/lite-api/settings.ts +++ b/packages/firestore/src/lite-api/settings.ts @@ -190,6 +190,12 @@ export class FirestoreSettingsImpl { this.experimental = cloneExperimentalOptions(settings.experimental ?? {}); validateExperimentalOptions(this.experimental); + console.trace( + 'zzyzx settings.experimental:', + settings.experimental, + ' this.experimental:', + this.experimental + ); } isEqual(other: FirestoreSettingsImpl): boolean { diff --git a/packages/firestore/src/register.ts b/packages/firestore/src/register.ts index 82b450b3834..33ea93fe81f 100644 --- a/packages/firestore/src/register.ts +++ b/packages/firestore/src/register.ts @@ -54,6 +54,7 @@ export function registerFirestore( app ); settings = { useFetchStreams, ...settings }; + console.trace('zzyzx registerFirestore() settings:', settings); firestoreInstance._setSettings(settings); return firestoreInstance; }, diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index 9642e03335b..a4e9d69aca8 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -189,11 +189,6 @@ class RemoteStoreImpl implements RemoteStore { readonly sendWriteRequestsDelayMs: number | null ) { if (sendWriteRequestsDelayMs !== null) { - debugAssert( - Number.isInteger(sendWriteRequestsDelayMs), - `invalid sendWriteRequestsDelayMs: ${sendWriteRequestsDelayMs} ` + - '(must be an integer, not a fractional amount)' - ); debugAssert( sendWriteRequestsDelayMs > 0, `invalid sendWriteRequestsDelayMs: ${sendWriteRequestsDelayMs} ` + @@ -769,10 +764,7 @@ function addToWritePipeline( writeRequestSent: false }); - if ( - remoteStoreImpl.sendWriteRequestsDelayMs === null || - !canAddToWritePipeline(remoteStoreImpl) - ) { + if (remoteStoreImpl.sendWriteRequestsDelayMs === null) { remoteStoreImpl.sendWriteRequestsOperation?.cancel(); remoteStoreImpl.sendWriteRequestsOperation = null; sendWriteRequestsFromPipeline(remoteStoreImpl); From 686c25d8761e14f060b4567b3c0dea5c4171651f Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 7 Apr 2025 19:19:17 +0000 Subject: [PATCH 07/11] remove console logs --- packages/firestore/src/lite-api/settings.ts | 6 ------ packages/firestore/src/register.ts | 1 - 2 files changed, 7 deletions(-) diff --git a/packages/firestore/src/lite-api/settings.ts b/packages/firestore/src/lite-api/settings.ts index b3a50b86b72..2bdc3ebc2f9 100644 --- a/packages/firestore/src/lite-api/settings.ts +++ b/packages/firestore/src/lite-api/settings.ts @@ -190,12 +190,6 @@ export class FirestoreSettingsImpl { this.experimental = cloneExperimentalOptions(settings.experimental ?? {}); validateExperimentalOptions(this.experimental); - console.trace( - 'zzyzx settings.experimental:', - settings.experimental, - ' this.experimental:', - this.experimental - ); } isEqual(other: FirestoreSettingsImpl): boolean { diff --git a/packages/firestore/src/register.ts b/packages/firestore/src/register.ts index 33ea93fe81f..82b450b3834 100644 --- a/packages/firestore/src/register.ts +++ b/packages/firestore/src/register.ts @@ -54,7 +54,6 @@ export function registerFirestore( app ); settings = { useFetchStreams, ...settings }; - console.trace('zzyzx registerFirestore() settings:', settings); firestoreInstance._setSettings(settings); return firestoreInstance; }, From 53ce817bbd3a412d54cd6f57961a526a8f0fffa3 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 7 Apr 2025 19:37:35 +0000 Subject: [PATCH 08/11] send write requests early if the pipeline is full and contains only unsent write requests --- packages/firestore/src/api/experimental_options.ts | 4 ++-- packages/firestore/src/lite-api/settings.ts | 4 +++- packages/firestore/src/remote/remote_store.ts | 9 ++++++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/firestore/src/api/experimental_options.ts b/packages/firestore/src/api/experimental_options.ts index 3fe711eb870..a2862d742cd 100644 --- a/packages/firestore/src/api/experimental_options.ts +++ b/packages/firestore/src/api/experimental_options.ts @@ -38,8 +38,8 @@ export interface ExperimentalOptions { * _could_ greatly reduce the number of distinct HTTP requests that are used. * * The value must be an integer value strictly greater than zero and less than - * or equal to 2000 (2 seconds). A value of `200` is a good starting point to - * minimize write latency yet still enable a some amount of batching. + * or equal to 10000 (10 seconds). A value of `200` is a good starting point + * to minimize write latency yet still enable some amount of batching. * * See https://github.com/firebase/firebase-js-sdk/issues/5971 for rationale * and background information that motivated this option. diff --git a/packages/firestore/src/lite-api/settings.ts b/packages/firestore/src/lite-api/settings.ts index 2bdc3ebc2f9..f2c5af1ef2c 100644 --- a/packages/firestore/src/lite-api/settings.ts +++ b/packages/firestore/src/lite-api/settings.ts @@ -55,7 +55,9 @@ const MAX_LONG_POLLING_TIMEOUT_SECONDS = 30; // Whether long-polling auto-detected is enabled by default. const DEFAULT_AUTO_DETECT_LONG_POLLING = true; -const MAX_SEND_WRITE_REQUEST_DELAY_MS = 2000; +// Set some maximum value for `sendWriteRequestsDelayMs` to avoid it being set +// to a value so large that it appears that write requests are never being sent. +const MAX_SEND_WRITE_REQUEST_DELAY_MS = 10000; /** * Specifies custom configurations for your Cloud Firestore instance. diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index a4e9d69aca8..8366150f8a0 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -764,7 +764,14 @@ function addToWritePipeline( writeRequestSent: false }); - if (remoteStoreImpl.sendWriteRequestsDelayMs === null) { + const writePipelineContainsOnlyUnsentWriteRequests = + remoteStoreImpl.writePipeline.every(entry => !entry.writeRequestSent); + + if ( + remoteStoreImpl.sendWriteRequestsDelayMs === null || + (!canAddToWritePipeline(remoteStoreImpl) && + writePipelineContainsOnlyUnsentWriteRequests) + ) { remoteStoreImpl.sendWriteRequestsOperation?.cancel(); remoteStoreImpl.sendWriteRequestsOperation = null; sendWriteRequestsFromPipeline(remoteStoreImpl); From 6fa6dc840619aa70187969b9708c8fc77a54a741 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 7 Apr 2025 19:41:40 +0000 Subject: [PATCH 09/11] yarn lint:fix --- packages/firestore/src/api/settings.ts | 2 +- packages/firestore/src/lite-api/settings.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/firestore/src/api/settings.ts b/packages/firestore/src/api/settings.ts index c5381608abf..5155be691bd 100644 --- a/packages/firestore/src/api/settings.ts +++ b/packages/firestore/src/api/settings.ts @@ -18,8 +18,8 @@ import { FirestoreSettings as LiteSettings } from '../lite-api/settings'; import { FirestoreLocalCache } from './cache_config'; -import { ExperimentalLongPollingOptions } from './long_polling_options'; import { ExperimentalOptions } from './experimental_options'; +import { ExperimentalLongPollingOptions } from './long_polling_options'; export { DEFAULT_HOST } from '../lite-api/settings'; diff --git a/packages/firestore/src/lite-api/settings.ts b/packages/firestore/src/lite-api/settings.ts index f2c5af1ef2c..a8b9d149a2f 100644 --- a/packages/firestore/src/lite-api/settings.ts +++ b/packages/firestore/src/lite-api/settings.ts @@ -19,16 +19,16 @@ import { EmulatorMockTokenOptions } from '@firebase/util'; import { FirestoreLocalCache } from '../api/cache_config'; import { CredentialsSettings } from '../api/credentials'; -import { - ExperimentalLongPollingOptions, - cloneLongPollingOptions, - longPollingOptionsEqual -} from '../api/long_polling_options'; import { ExperimentalOptions, cloneExperimentalOptions, experimentalOptionsEqual } from '../api/experimental_options'; +import { + ExperimentalLongPollingOptions, + cloneLongPollingOptions, + longPollingOptionsEqual +} from '../api/long_polling_options'; import { LRU_COLLECTION_DISABLED, LRU_DEFAULT_CACHE_SIZE_BYTES From a03d5fbcd482f27c7979633c1e115eeb4071d15c Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 7 Apr 2025 20:29:39 +0000 Subject: [PATCH 10/11] yarn docgen:all --- common/api-review/firestore.api.md | 6 +++ docs-devsite/_toc.yaml | 2 + .../firestore_.experimentaloptions.md | 45 +++++++++++++++++++ docs-devsite/firestore_.firestoresettings.md | 11 +++++ docs-devsite/firestore_.md | 1 + 5 files changed, 65 insertions(+) create mode 100644 docs-devsite/firestore_.experimentaloptions.md diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index 34b56b97f21..b588fa2389d 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -211,6 +211,11 @@ export interface ExperimentalLongPollingOptions { timeoutSeconds?: number; } +// @public +export interface ExperimentalOptions { + sendWriteRequestsDelayMs?: number; +} + // @public export class FieldPath { constructor(...fieldNames: string[]); @@ -252,6 +257,7 @@ export type FirestoreLocalCache = MemoryLocalCache | PersistentLocalCache; // @public export interface FirestoreSettings { cacheSizeBytes?: number; + experimental?: ExperimentalOptions; experimentalAutoDetectLongPolling?: boolean; experimentalForceLongPolling?: boolean; experimentalLongPollingOptions?: ExperimentalLongPollingOptions; diff --git a/docs-devsite/_toc.yaml b/docs-devsite/_toc.yaml index 665222edb9d..383c2e92736 100644 --- a/docs-devsite/_toc.yaml +++ b/docs-devsite/_toc.yaml @@ -221,6 +221,8 @@ toc: path: /docs/reference/js/firestore_.documentsnapshot.md - title: ExperimentalLongPollingOptions path: /docs/reference/js/firestore_.experimentallongpollingoptions.md + - title: ExperimentalOptions + path: /docs/reference/js/firestore_.experimentaloptions.md - title: FieldPath path: /docs/reference/js/firestore_.fieldpath.md - title: FieldValue diff --git a/docs-devsite/firestore_.experimentaloptions.md b/docs-devsite/firestore_.experimentaloptions.md new file mode 100644 index 00000000000..51a1c846945 --- /dev/null +++ b/docs-devsite/firestore_.experimentaloptions.md @@ -0,0 +1,45 @@ +Project: /docs/reference/js/_project.yaml +Book: /docs/reference/_book.yaml +page_type: reference + +{% comment %} +DO NOT EDIT THIS FILE! +This is generated by the JS SDK team, and any local changes will be +overwritten. Changes should be made in the source code at +https://github.com/firebase/firebase-js-sdk +{% endcomment %} + +# ExperimentalOptions interface +Experimental options to configure the Firestore SDK. + +Note: This interface is "experimental" and is subject to change. + +See `FirestoreSettings.experimental`. + +Signature: + +```typescript +export declare interface ExperimentalOptions +``` + +## Properties + +| Property | Type | Description | +| --- | --- | --- | +| [sendWriteRequestsDelayMs](./firestore_.experimentaloptions.md#experimentaloptionssendwriterequestsdelayms) | number | The maximum amount of time, in milliseconds, to wait before sending a Firestore "write" request to the backend. If undefined then do not delay at all.A delay can be useful because it enables the in-memory "write pipeline" to gather together multiple write requests and send them in a single HTTP request to the backend, rather than one HTTP request per write request, as is done when by default, or when this property is undefined. Note that there is a hardcoded limit to the number of write requests that are sent at once, so setting a very large value for this property will not necessarily cause \_all\_ write requests to be sent in a single HTTP request; however, it \_could\_ greatly reduce the number of distinct HTTP requests that are used.The value must be an integer value strictly greater than zero and less than or equal to 10000 (10 seconds). A value of 200 is a good starting point to minimize write latency yet still enable some amount of batching.See https://github.com/firebase/firebase-js-sdk/issues/5971 for rationale and background information that motivated this option. | + +## ExperimentalOptions.sendWriteRequestsDelayMs + +The maximum amount of time, in milliseconds, to wait before sending a Firestore "write" request to the backend. If `undefined` then do not delay at all. + +A delay can be useful because it enables the in-memory "write pipeline" to gather together multiple write requests and send them in a single HTTP request to the backend, rather than one HTTP request per write request, as is done when by default, or when this property is `undefined`. Note that there is a hardcoded limit to the number of write requests that are sent at once, so setting a very large value for this property will not necessarily cause \_all\_ write requests to be sent in a single HTTP request; however, it \_could\_ greatly reduce the number of distinct HTTP requests that are used. + +The value must be an integer value strictly greater than zero and less than or equal to 10000 (10 seconds). A value of `200` is a good starting point to minimize write latency yet still enable some amount of batching. + +See https://github.com/firebase/firebase-js-sdk/issues/5971 for rationale and background information that motivated this option. + +Signature: + +```typescript +sendWriteRequestsDelayMs?: number; +``` diff --git a/docs-devsite/firestore_.firestoresettings.md b/docs-devsite/firestore_.firestoresettings.md index 6d24d583175..711e5b76bca 100644 --- a/docs-devsite/firestore_.firestoresettings.md +++ b/docs-devsite/firestore_.firestoresettings.md @@ -23,6 +23,7 @@ export declare interface FirestoreSettings | Property | Type | Description | | --- | --- | --- | | [cacheSizeBytes](./firestore_.firestoresettings.md#firestoresettingscachesizebytes) | number | NOTE: This field will be deprecated in a future major release. Use cache field instead to specify cache size, and other cache configurations.An approximate cache size threshold for the on-disk data. If the cache grows beyond this size, Firestore will start removing data that hasn't been recently used. The size is not a guarantee that the cache will stay below that size, only that if the cache exceeds the given size, cleanup will be attempted.The default value is 40 MB. The threshold must be set to at least 1 MB, and can be set to CACHE_SIZE_UNLIMITED to disable garbage collection. | +| [experimental](./firestore_.firestoresettings.md#firestoresettingsexperimental) | [ExperimentalOptions](./firestore_.experimentaloptions.md#experimentaloptions_interface) | Options that are "experimental", meaning that their semantics are subject to change at any time without notice, up to and including complete removal. | | [experimentalAutoDetectLongPolling](./firestore_.firestoresettings.md#firestoresettingsexperimentalautodetectlongpolling) | boolean | Configures the SDK's underlying transport (WebChannel) to automatically detect if long-polling should be used. This is very similar to experimentalForceLongPolling, but only uses long-polling if required.After having had a default value of false since its inception in 2019, the default value of this setting was changed in May 2023 to true in v9.22.0 of the Firebase JavaScript SDK. That is, auto-detection of long polling is now enabled by default. To disable it, set this setting to false, and please open a GitHub issue to share the problems that motivated you disabling long-polling auto-detection.This setting cannot be used in a Node.js environment. | | [experimentalForceLongPolling](./firestore_.firestoresettings.md#firestoresettingsexperimentalforcelongpolling) | boolean | Forces the SDK’s underlying network transport (WebChannel) to use long-polling. Each response from the backend will be closed immediately after the backend sends data (by default responses are kept open in case the backend has more data to send). This avoids incompatibility issues with certain proxies, antivirus software, etc. that incorrectly buffer traffic indefinitely. Use of this option will cause some performance degradation though.This setting cannot be used with experimentalAutoDetectLongPolling and may be removed in a future release. If you find yourself using it to work around a specific network reliability issue, please tell us about it in https://github.com/firebase/firebase-js-sdk/issues/1674.This setting cannot be used in a Node.js environment. | | [experimentalLongPollingOptions](./firestore_.firestoresettings.md#firestoresettingsexperimentallongpollingoptions) | [ExperimentalLongPollingOptions](./firestore_.experimentallongpollingoptions.md#experimentallongpollingoptions_interface) | Options that configure the SDK’s underlying network transport (WebChannel) when long-polling is used.These options are only used if experimentalForceLongPolling is true or if experimentalAutoDetectLongPolling is true and the auto-detection determined that long-polling was needed. Otherwise, these options have no effect. | @@ -45,6 +46,16 @@ The default value is 40 MB. The threshold must be set to at least 1 MB, and can cacheSizeBytes?: number; ``` +## FirestoreSettings.experimental + +Options that are "experimental", meaning that their semantics are subject to change at any time without notice, up to and including complete removal. + +Signature: + +```typescript +experimental?: ExperimentalOptions; +``` + ## FirestoreSettings.experimentalAutoDetectLongPolling Configures the SDK's underlying transport (WebChannel) to automatically detect if long-polling should be used. This is very similar to `experimentalForceLongPolling`, but only uses long-polling if required. diff --git a/docs-devsite/firestore_.md b/docs-devsite/firestore_.md index 91d21e32708..2e87bc92027 100644 --- a/docs-devsite/firestore_.md +++ b/docs-devsite/firestore_.md @@ -168,6 +168,7 @@ https://github.com/firebase/firebase-js-sdk | [DocumentChange](./firestore_.documentchange.md#documentchange_interface) | A DocumentChange represents a change to the documents matching a query. It contains the document affected and the type of change that occurred. | | [DocumentData](./firestore_.documentdata.md#documentdata_interface) | Document data (for use with [setDoc()](./firestore_lite.md#setdoc_ee215ad)) consists of fields mapped to values. | | [ExperimentalLongPollingOptions](./firestore_.experimentallongpollingoptions.md#experimentallongpollingoptions_interface) | Options that configure the SDK’s underlying network transport (WebChannel) when long-polling is used.Note: This interface is "experimental" and is subject to change.See FirestoreSettings.experimentalAutoDetectLongPolling, FirestoreSettings.experimentalForceLongPolling, and FirestoreSettings.experimentalLongPollingOptions. | +| [ExperimentalOptions](./firestore_.experimentaloptions.md#experimentaloptions_interface) | Experimental options to configure the Firestore SDK.Note: This interface is "experimental" and is subject to change.See FirestoreSettings.experimental. | | [FirestoreDataConverter](./firestore_.firestoredataconverter.md#firestoredataconverter_interface) | Converter used by withConverter() to transform user objects of type AppModelType into Firestore data of type DbModelType.Using the converter allows you to specify generic type arguments when storing and retrieving objects from Firestore.In this context, an "AppModel" is a class that is used in an application to package together related information and functionality. Such a class could, for example, have properties with complex, nested data types, properties used for memoization, properties of types not supported by Firestore (such as symbol and bigint), and helper functions that perform compound operations. Such classes are not suitable and/or possible to store into a Firestore database. Instead, instances of such classes need to be converted to "plain old JavaScript objects" (POJOs) with exclusively primitive properties, potentially nested inside other POJOs or arrays of POJOs. In this context, this type is referred to as the "DbModel" and would be an object suitable for persisting into Firestore. For convenience, applications can implement FirestoreDataConverter and register the converter with Firestore objects, such as DocumentReference or Query, to automatically convert AppModel to DbModel when storing into Firestore, and convert DbModel to AppModel when retrieving from Firestore. | | [FirestoreSettings](./firestore_.firestoresettings.md#firestoresettings_interface) | Specifies custom configurations for your Cloud Firestore instance. You must set these before invoking any other methods. | | [Index](./firestore_.index.md#index_interface) | (Public Preview) The SDK definition of a Firestore index. | From 74d6f2e75635a0aade30c1b702d5d98dca93789d Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 7 Apr 2025 20:30:03 +0000 Subject: [PATCH 11/11] remote_store.ts: cancel the async task upon shutdown --- packages/firestore/src/remote/remote_store.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index 8366150f8a0..2ed4043084c 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -283,6 +283,8 @@ export async function remoteStoreShutdown( ): Promise { const remoteStoreImpl = debugCast(remoteStore, RemoteStoreImpl); logDebug(LOG_TAG, 'RemoteStore shutting down.'); + remoteStoreImpl.sendWriteRequestsOperation?.cancel(); + remoteStoreImpl.sendWriteRequestsOperation = null; remoteStoreImpl.offlineCauses.add(OfflineCause.Shutdown); await disableNetworkInternal(remoteStoreImpl); remoteStoreImpl.connectivityMonitor.shutdown();