Change maxSockets behavior/defaults

Fixes #702.

Author: pluma

CHANGELOG.md

@@ -24,6 +24,23 @@ This driver uses semantic versioning:
   sockets expiring, especially with larger connection pools and infrequent
   requests.
 
+- Removed `keepAlive`-specific throughput optimization
+
+  Previously arangojs would allow `agentOptions.maxSockets * 2` concurrent
+  requests, to optimize socket reuse by avoiding idle time. This behavior
+  could trigger deadlocks when attempting to perform multiple transactions
+  in parallel and only marginally improved throughput in some high-load
+  scenarios. The connection pool size now always reflects the value set in
+  `agentOptions.maxSockets` regardless of whether `keepAlive` is enabled.
+
+- Changed `agentOptions.maxSockets` default value when using `ROUND_ROBIN`
+
+  As the connection pool is shared across all server connections when using
+  `ROUND_ROBIN` load balancing, the default value of `3` is too limiting for
+  most scenarios involving multiple coordinators. When passing multiple URLs
+  via the `url` option and specifying `ROUND_ROBIN` load balancing, arangojs
+  will now default this value to `url.length * 3` instead.
+
 ## [7.2.0] - 2020-12-02
 
 ### Added

README.md

@@ -9,11 +9,11 @@ The official ArangoDB JavaScript client for Node.js and the browser.
 
 ## Links
 
-* [API Documentation](https://arangodb.github.io/arangojs/latest/modules/_index_.html)
+- [API Documentation](https://arangodb.github.io/arangojs/latest/modules/_index_.html)
 
-* [Changelog](https://arangodb.github.io/arangojs/CHANGELOG)
+- [Changelog](https://arangodb.github.io/arangojs/CHANGELOG)
 
-* [Migration Guide](http://arangodb.github.io/arangojs/MIGRATING)
+- [Migration Guide](http://arangodb.github.io/arangojs/MIGRATING)
 
 ## Install
 
@@ -321,7 +321,7 @@ HTTPS certificate validation entirely, but note this has
 When using arangojs in the browser, self-signed HTTPS certificates need to
 be trusted by the browser or use a trusted root certificate.
 
-### Streaming transactions
+### Streaming transactions leak
 
 When using the `transaction.step` method it is important to be aware of the
 limitations of what a callback passed to this method is allowed to do.
@@ -343,6 +343,22 @@ await trx.step(() => collection.save(doc2));
 
 Please refer to the documentation of this method for additional examples.
 
+### Streaming transactions timeout in cluster
+
+Transactions have
+[different guarantees](https://www.arangodb.com/docs/stable/transactions-limitations.html#in-clusters)
+in a cluster.
+
+When using arangojs in a cluster with load balancing, you may need to adjust
+the value of `agentOptions.maxSockets` to accommodate the number of transactions
+you need to be able to run in parallel. The default value is likely to be too
+low for most cluster scenarios involving frequent streaming transactions.
+
+**Note**: When using a high value for `agentOptions.maxSockets` you may have
+to adjust the maximum number of threads in the ArangoDB configuration using
+[the `server.maximal-threads` option](https://www.arangodb.com/docs/3.7/programs-arangod-server.html#server-threads)
+to support larger numbers of concurrent transactions on the server side.
+
 ## License
 
 The Apache License, Version 2.0. For more information, see the accompanying

src/connection.ts

@@ -432,10 +432,16 @@ export type Config = {
    * module when using arangojs in the browser). This will be ignored if
    * `agent` is also provided.
    *
-   * The option `maxSockets` can also be used to limit how many requests
+   * The option `maxSockets` is also used to limit how many requests
    * arangojs will perform concurrently. The maximum number of requests is
-   * equal to `maxSockets * 2` with `keepAlive: true` or equal to `maxSockets`
-   * with `keepAlive: false` (or in the browser).
+   * equal to `maxSockets`.
+   *
+   * **Note:** arangojs will limit the number of concurrent requests based on
+   * this value even if an `agent` is provided.
+   *
+   * **Note:** when using `ROUND_ROBIN` load balancing and passing an array of
+   * URLs in the `url` option, the default value of `maxSockets` will be set
+   * to `3 * url.length` instead of `3`.
    *
    * Default (Node.js): `{ maxSockets: 3, keepAlive: true, keepAliveMsecs: 1000 }`
    *
@@ -509,40 +515,41 @@ export class Connection {
    * @hidden
    */
   constructor(config: Omit<Config, "databaseName"> = {}) {
+    const URLS = config.url
+      ? Array.isArray(config.url)
+        ? config.url
+        : [config.url]
+      : ["http://localhost:8529"];
+    const MAX_SOCKETS =
+      3 * (config.loadBalancingStrategy === "ROUND_ROBIN" ? URLS.length : 1);
+
     if (config.arangoVersion !== undefined) {
       this._arangoVersion = config.arangoVersion;
     }
     this._agent = config.agent;
     this._agentOptions = isBrowser
-      ? { maxSockets: 3, ...config.agentOptions }
+      ? { maxSockets: MAX_SOCKETS, ...config.agentOptions }
       : {
-          maxSockets: 3,
+          maxSockets: MAX_SOCKETS,
           keepAlive: true,
           keepAliveMsecs: 1000,
           scheduling: "lifo",
           ...config.agentOptions,
         };
     this._maxTasks = this._agentOptions.maxSockets;
-    if (this._agentOptions.keepAlive) this._maxTasks *= 2;
-
     this._headers = { ...config.headers };
-    this._loadBalancingStrategy = config.loadBalancingStrategy || "NONE";
+    this._loadBalancingStrategy = config.loadBalancingStrategy ?? "NONE";
     this._useFailOver = this._loadBalancingStrategy !== "ROUND_ROBIN";
     this._precaptureStackTraces = Boolean(config.precaptureStackTraces);
     if (config.maxRetries === false) {
       this._shouldRetry = false;
       this._maxRetries = 0;
     } else {
       this._shouldRetry = true;
-      this._maxRetries = config.maxRetries || 0;
+      this._maxRetries = config.maxRetries ?? 0;
     }
 
-    const urls = config.url
-      ? Array.isArray(config.url)
-        ? config.url
-        : [config.url]
-      : ["http://localhost:8529"];
-    this.addToHostList(urls);
+    this.addToHostList(URLS);
 
     if (config.auth) {
       if (isBearerAuth(config.auth)) {