Load balancing #326

adespawn · 2025-11-21T11:47:01Z

This PR adds a new load balancing policy, as well as attempts to add support for some of the old policies.

Right now, this is not tested, as all of the current integration tests rely on info.queried hosts, which are not available in the rust driver.

See: scylladb#297 (comment)

Update query options construction to be a method on the Client wrapper, instead of a standalone function. This change will allow for adding load balancing policies, which are configured at the client level, in the DSx driver, and on the statement level in the Rust driver.

This adds a rust driver default load balancing policy. This can be configured through JS at the session construction, and will be used for all queries. Support for configuring it through the driver interface will be added in the following commits.

Copilot

Pull Request Overview

This PR introduces load balancing support by integrating the Rust driver's default load balancing policy into the JavaScript driver. It adds a new NewDefaultLoadBalancingPolicy class and LoadBalancingConfig configuration object, while deprecating the old DefaultLoadBalancingPolicy implementation. The PR also removes the deprecated WhiteListPolicy and updates the defaultLoadBalancingPolicy() function to no longer accept a localDc parameter.

Key changes:

Adds Rust-backed load balancing policy implementation with support for datacenter/rack awareness, token awareness, and datacenter failover
Refactors batch creation methods to be instance methods on SessionWrapper to enable per-session load balancing policy configuration
Updates existing policies (RoundRobinPolicy, DCAwareRoundRobinPolicy, TokenAwarePolicy) to provide Rust configuration via getRustConfiguration()

Reviewed Changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`src/session.rs`	Adds `LoadBalancingConfig` struct, `session_load_balancing_policy` field, and refactors statement/batch option application to be instance methods that apply the session's load balancing policy
`src/utils/from_napi_obj.rs`	Updates macro to support trailing commas in field definitions
`src/utils/to_napi_obj.rs`	Updates macro to support trailing commas in field definitions
`src/requests/request.rs`	Adds trailing comma to `QueryOptionsObj` definition for consistency
`src/metadata/host.rs`	Adds trailing comma to `HostWrapper` definition for consistency
`lib/policies/load-balancing.js`	Adds `NewDefaultLoadBalancingPolicy` and `LoadBalancingConfig` classes, adds `LoadBalancingRustImplemented` base class, updates existing policies to support Rust configuration, removes deprecated `WhiteListPolicy`
`lib/policies/index.js`	Updates `defaultLoadBalancingPolicy()` to return `NewDefaultLoadBalancingPolicy` and removes `localDc` parameter
`lib/policies/index.d.ts`	Updates TypeScript definition for `defaultLoadBalancingPolicy()` to remove optional parameter
`lib/client.js`	Changes batch creation from static functions to instance methods on `rustClient`
`lib/client-options.js`	Adds load balancing configuration extraction and validation in `setRustOptions()`
`test/unit/default-policies-tests.js`	Adds basic test for new default load balancing policy
`test/unit-broken/default-load-balancing-policy-tests.js`	Removes tests for old implementation moved to broken tests directory
`test/unit-not-supported/api-tests.js`	Removes `WhiteListPolicy` from API surface tests
`test/typescript/policy-tests.ts`	Removes usage of `WhiteListPolicy` and `localDc` parameter
`test/test-helper.js`	Restores `client.hosts` functionality check (previously commented out)

Comments suppressed due to low confidence (1)

lib/policies/load-balancing.js:16

[nitpick] Inconsistent empty constructor formatting. The codebase uses constructor() {} without a space between the braces. This should be constructor() {} to match the existing style in files like lib/execution-options.js:27, lib/policies/address-resolution.js:27, lib/policies/retry.js:11, etc.

/**

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-21T11:53:25Z

src/session.rs

+
+#[rustfmt::skip] // fmt splits the struct definition into multiple lines
+define_js_to_rust_convertible_object!(
+ LoadBalancingConfig {


[nitpick] Inconsistent indentation. The opening brace should be aligned with the macro name. The line should start with no leading space, like:

define_js_to_rust_convertible_object!( LoadBalancingConfig {

Suggested change

LoadBalancingConfig {

LoadBalancingConfig {

Copilot · 2025-11-21T11:53:25Z

lib/client-options.js

+                // We will catch this error when:
+                //  - The policy does not implement getRustConfiguration (someone provided policy that does not inherit from LoadBalancingPolicy)
+                //  - The policy implements getRustConfiguration but throws "Currently this policy is not supported by the driver"
+                //  - Some other obscure error, like node deciding it has a bad day and just want't to crash


Typo in comment: "want't" should be "wants". Also, the comment is somewhat informal. Consider rewording to: "- Some other unexpected error occurs"

Suggested change

// - Some other obscure error, like node deciding it has a bad day and just want't to crash

// - Some other unexpected error occurs

This is a policy that is a wrapper for the Rust driver default load balancing policy, The goal for this policy is to hold configuration options, that will be passed to the Rust, during the session creation.

This policy was already deprecated in the DSx driver

As the old default load balancing policy is not supported, ans starting form the next commit, we will use the load balancing policy, this commit updates the default policy, to the supported one.

This commit attempts to to map the old policies to appropriate rust default policy configuration. currently this is not tested, as all of the current integration tests rely on info.queried hosts, which is not available in the rust driver.

wprzytula · 2025-11-21T13:34:23Z

Right now, this is not tested, as all of the current integration tests rely on info.queried hosts, which are not available in the rust driver.

I believe you can used HistoryListener capabilities of Rust Driver to emulate info.queriedHosts. Especially StructuredHistory should contain required information.

wprzytula · 2025-11-21T14:18:25Z

src/session.rs

+
+#[rustfmt::skip] // fmt splits the struct definition into multiple lines
+define_js_to_rust_convertible_object!(
+ LoadBalancingConfig {


wprzytula · 2025-11-21T14:21:42Z

src/session.rs

 pub struct SessionWrapper {
    pub(crate) inner: CachingSession,
+    pub(crate) session_load_balancing_policy: Option<Arc<dyn LoadBalancingPolicy>>,
 }


🤔 Session-level LBP should be configured on the Session-level execution profile.

wprzytula · 2025-11-21T14:57:11Z

src/session.rs

+            if let Some(prefer_dc) = &config.prefer_datacenter {
+                if let Some(prefer_rack) = &config.prefer_rack {
+                    builder = builder
+                        .prefer_datacenter_and_rack(prefer_dc.to_owned(), prefer_rack.to_owned());
+                } else {
+                    builder = builder.prefer_datacenter(prefer_dc.to_owned());
+                }
+            } else if config.prefer_rack.is_some() {
+                return Err(js_error(
+                    "rack preference cannot be set without setting dc preference",
+                ));
+            }


♻️ Prefer a single match over pair of both config params instead of nested ifs.

wprzytula · 2025-11-21T14:58:59Z

lib/policies/index.d.ts

+    class NewDefaultLoadBalancingPolicy extends LoadBalancingPolicy {
+      constructor(config?: LoadBalancingConfig);
+    }


This will at some point no longer be new.
I'd call it simply DefaultLoadBalancingPolicy.

Ugh, but there already is something called DefaultLoadBalancingPolicy...

wprzytula · 2025-11-21T21:50:48Z

lib/policies/load-balancing.js

+ * It can be configured to be datacenter-aware, rack-aware and token-aware.
+ * Datacenter failover (sending query to a node from a remote datacenter)
+ * for queries with non local consistency mode is also supported.


AFAIR we changed the logic of DC failover to be possible also for queries with local consistency.
Please update the docs here. If you could be that kind and update Rust docs as well, it would be great 🙏

@Lorak-mmk changed Rust logic in 04a3751cfe635222e0d93c652e2af243c9a6fd3d, but docs must have remained obsolete.

wprzytula · 2025-11-21T21:55:28Z

lib/policies/load-balancing.js

+     * When it is set, the policy will prefer to return alive remote replicas
+     * if datacenter failover is permitted and possible due to consistency
+     * constraints.
+     *
+     *  @type {boolean?}
+     */
+    permitDcFailover;


There are no more possible consistency constraints after the change in 04a3751cfe635222e0d93c652e2af243c9a6fd3d in Rust Driver.

wprzytula · 2025-11-21T22:01:05Z

lib/policies/index.js

 */
-exports.defaultLoadBalancingPolicy = function (localDc) {
-    return new loadBalancing.DefaultLoadBalancingPolicy(localDc);
+exports.defaultLoadBalancingPolicy = function () {
+    return new loadBalancing.NewDefaultLoadBalancingPolicy();
 };


💭 For maintaining as much compatibility as possible, shouldn't we accept localDc arg? Dunno.

wprzytula · 2025-11-21T22:01:54Z

lib/policies/load-balancing.js


    /**
-     * @param {LoadBalancingConfig} config
+     * @param {LoadBalancingConfig} [config]


♻️ This should belong to an earlier commit, right?

wprzytula · 2025-11-21T22:03:29Z

lib/client-options.js

+                throw new Error(
+                    "This load balancing policy does not appear to be supported by the driver. Root cause: " +
+                        e.message,
+                );


🔧 Can we include the policy's class name in the message?

wprzytula · 2025-11-21T22:04:03Z

lib/policies/load-balancing.js

+    /**
+     * @returns {LoadBalancingConfig}
+     * @package
+     */
+    getRustConfiguration() {
+        // This error will be thrown by all policies, that do not override this method.
+        throw new Error("Currently this policy is not supported by the driver");
+    }


Suggested change

/**

* @returns {LoadBalancingConfig}

* @package

*/

getRustConfiguration() {

// This error will be thrown by all policies, that do not override this method.

throw new Error("Currently this policy is not supported by the driver");

}

/**

* @returns {LoadBalancingConfig}

* @package

*/

getRustConfiguration() {

// This error will be thrown by all policies, that do not override this method.

throw new Error("Currently this load balancing policy is not supported by the driver");

}

adespawn added 3 commits November 21, 2025 12:01

Add trailing commas to struct macros

406748a

See: scylladb#297 (comment)

adespawn requested a review from Copilot November 21, 2025 11:47

Copilot started reviewing on behalf of adespawn November 21, 2025 11:47 View session

adespawn force-pushed the load-balancing branch from 39510f3 to 888b653 Compare November 21, 2025 11:50

adespawn self-assigned this Nov 21, 2025

adespawn added this to the GA milestone Nov 21, 2025

Copilot finished reviewing on behalf of adespawn November 21, 2025 11:52

adespawn mentioned this pull request Nov 21, 2025

Implement load balancing policies #186

Open

5 tasks

Copilot AI reviewed Nov 21, 2025

View reviewed changes

adespawn added 6 commits November 21, 2025 12:59

Add a new load balancing policy

e91d91a

This is a policy that is a wrapper for the Rust driver default load balancing policy, The goal for this policy is to hold configuration options, that will be passed to the Rust, during the session creation.

Remove WhiteListPolicy from the API

0720591

This policy was already deprecated in the DSx driver

Update the default load balancing policy

3243b21

As the old default load balancing policy is not supported, ans starting form the next commit, we will use the load balancing policy, this commit updates the default policy, to the supported one.

Add configuration for load balancing policies

f261b16

Adapt some of the old policies

7fe0952

This commit attempts to to map the old policies to appropriate rust default policy configuration. currently this is not tested, as all of the current integration tests rely on info.queried hosts, which is not available in the rust driver.

Enable turned off par of test helper

8025057

adespawn force-pushed the load-balancing branch from 888b653 to 8025057 Compare November 21, 2025 12:00

adespawn requested a review from wprzytula November 21, 2025 13:20

wprzytula requested changes Nov 21, 2025

View reviewed changes

	// - Some other obscure error, like node deciding it has a bad day and just want't to crash
	// - Some other unexpected error occurs

Load balancing #326

Are you sure you want to change the base?

Load balancing #326

Uh oh!

Conversation

adespawn commented Nov 21, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

wprzytula commented Nov 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants