fix: set default max ratelimiter throughput to 10k and update docs (#…

…1439)
googleapis · Mar 3, 2021 · c06fb3c · c06fb3c
1 parent c25f1d0
commit c06fb3c
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 20 deletions.
diff --git a/dev/src/bulk-writer.ts b/dev/src/bulk-writer.ts
@@ -53,22 +53,26 @@ const MAX_BATCH_SIZE = 20;
  * The starting maximum number of operations per second as allowed by the
  * 500/50/5 rule.
  *
- * https://cloud.google.com/datastore/docs/best-practices#ramping_up_traffic.
+ * https://firebase.google.com/docs/firestore/best-practices#ramping_up_traffic.
  */
-export const DEFAULT_STARTING_MAXIMUM_OPS_PER_SECOND = 500;
+export const DEFAULT_INITIAL_OPS_PER_SECOND_LIMIT = 500;
 
 /*!
- * The rate by which to increase the capacity as specified by the 500/50/5 rule.
+ * The maximum number of operations per second as allowed by the 500/50/5 rule.
+ * By default the rate limiter will not exceed this value.
  *
- * https://cloud.google.com/datastore/docs/best-practices#ramping_up_traffic.
+ * https://firebase.google.com/docs/firestore/best-practices#ramping_up_traffic.
+ */
+export const DEFAULT_MAXIMUM_OPS_PER_SECOND_LIMIT = 10000;
+
+/*!
+ * The rate by which to increase the capacity as specified by the 500/50/5 rule.
  */
 const RATE_LIMITER_MULTIPLIER = 1.5;
 
 /*!
  * How often the operations per second capacity should increase in milliseconds
  * as specified by the 500/50/5 rule.
- *
- * https://cloud.google.com/datastore/docs/best-practices#ramping_up_traffic.
  */
 const RATE_LIMITER_MULTIPLIER_MILLIS = 5 * 60 * 1000;
 
@@ -336,8 +340,8 @@ export class BulkWriter {
         Number.POSITIVE_INFINITY
       );
     } else {
-      let startingRate = DEFAULT_STARTING_MAXIMUM_OPS_PER_SECOND;
-      let maxRate = Number.POSITIVE_INFINITY;
+      let startingRate = DEFAULT_INITIAL_OPS_PER_SECOND_LIMIT;
+      let maxRate = DEFAULT_MAXIMUM_OPS_PER_SECOND_LIMIT;
 
       if (typeof options?.throttling !== 'boolean') {
         if (options?.throttling?.maxOpsPerSecond !== undefined) {

diff --git a/dev/src/index.ts b/dev/src/index.ts
@@ -728,12 +728,13 @@ export class Firestore implements firestore.Firestore {
    * multiple writes in parallel. Gradually ramps up writes as specified
    * by the 500/50/5 rule.
    *
-   * @see [500/50/5 Documentation]{@link https://cloud.google.com/datastore/docs/best-practices#ramping_up_traffic}
+   * If you pass [BulkWriterOptions]{@link BulkWriterOptions}, you can
+   * configure the throttling rates for the created BulkWriter.
    *
-   * @param {object=} options BulkWriter options.
-   * @param {boolean=} options.disableThrottling Whether to disable throttling
-   * as specified by the 500/50/5 rule.
-   * @returns {WriteBatch} A BulkWriter that operates on this Firestore
+   * @see [500/50/5 Documentation]{@link https://firebase.google.com/docs/firestore/best-practices#ramping_up_traffic}
+   *
+   * @param {BulkWriterOptions=} options BulkWriter options.
+   * @returns {BulkWriter} A BulkWriter that operates on this Firestore
    * client.
    *
    * @example

diff --git a/dev/src/rate-limiter.ts b/dev/src/rate-limiter.ts
@@ -26,7 +26,7 @@ import {logger} from './logger';
  *
  * RateLimiter can also implement a gradually increasing rate limit. This is
  * used to enforce the 500/50/5 rule
- * (https://cloud.google.com/datastore/docs/best-practices#ramping_up_traffic).
+ * (https://firebase.google.com/docs/firestore/best-practices#ramping_up_traffic).
  *
  * @private
  */

diff --git a/dev/test/bulk-writer.ts b/dev/test/bulk-writer.ts
@@ -29,7 +29,8 @@ import {
 import {setTimeoutHandler} from '../src/backoff';
 import {
   BulkWriterError,
-  DEFAULT_STARTING_MAXIMUM_OPS_PER_SECOND,
+  DEFAULT_INITIAL_OPS_PER_SECOND_LIMIT,
+  DEFAULT_MAXIMUM_OPS_PER_SECOND_LIMIT,
 } from '../src/bulk-writer';
 import {
   ApiOverride,
@@ -253,7 +254,7 @@ describe('BulkWriter', () => {
       });
       expect(bulkWriter._rateLimiter.availableTokens).to.equal(100);
       expect(bulkWriter._rateLimiter.maximumCapacity).to.equal(
-        Number.POSITIVE_INFINITY
+        DEFAULT_MAXIMUM_OPS_PER_SECOND_LIMIT
       );
 
       bulkWriter = firestore.bulkWriter({
@@ -264,18 +265,18 @@ describe('BulkWriter', () => {
 
       bulkWriter = firestore.bulkWriter();
       expect(bulkWriter._rateLimiter.availableTokens).to.equal(
-        DEFAULT_STARTING_MAXIMUM_OPS_PER_SECOND
+        DEFAULT_INITIAL_OPS_PER_SECOND_LIMIT
       );
       expect(bulkWriter._rateLimiter.maximumCapacity).to.equal(
-        Number.POSITIVE_INFINITY
+        DEFAULT_MAXIMUM_OPS_PER_SECOND_LIMIT
       );
 
       bulkWriter = firestore.bulkWriter({throttling: true});
       expect(bulkWriter._rateLimiter.availableTokens).to.equal(
-        DEFAULT_STARTING_MAXIMUM_OPS_PER_SECOND
+        DEFAULT_INITIAL_OPS_PER_SECOND_LIMIT
       );
       expect(bulkWriter._rateLimiter.maximumCapacity).to.equal(
-        Number.POSITIVE_INFINITY
+        DEFAULT_MAXIMUM_OPS_PER_SECOND_LIMIT
       );
 
       bulkWriter = firestore.bulkWriter({throttling: false});

diff --git a/types/firestore.d.ts b/types/firestore.d.ts
@@ -290,6 +290,8 @@ declare namespace FirebaseFirestore {
      * multiple writes in parallel. Gradually ramps up writes as specified
      * by the 500/50/5 rule.
      *
+     * @see https://firebase.google.com/docs/firestore/best-practices#ramping_up_traffic
+     *
      * @param options An options object used to configure the throttling
      * behavior for the underlying BulkWriter.
      */
@@ -682,6 +684,8 @@ declare namespace FirebaseFirestore {
      * the defaults by setting it to `false` to disable throttling, or by
      * setting the config values to enable throttling with the provided values.
      *
+     * @see https://firebase.google.com/docs/firestore/best-practices#ramping_up_traffic
+     *
      * @param initialOpsPerSecond The initial maximum number of operations per
      * second allowed by the throttler. If this field is not set, the default
      * is 500 operations per second.