Add actions to list nodes and retrieve node metrics by CGodiksen · Pull Request #418 · ModelarData/ModelarDB-RS

CGodiksen · 2026-08-12T11:14:40Z

This PR adds support for two new actions, ListNodes and NodeMetrics. ListNodes lists all the nodes in the cluster, or the node itself if it is a single node deployment. NodeMetrics returns a collection of metrics that show general system metrics such as CPU, disk, and memory usage, and ModelarDB specific metrics such as storage engine memory usage.

Methods have also been added to Client in modelardb_embedded to support node related operations on the client. Note that the client still implements the Operations trait for functionality that is shared between DataFolder and Client so these new methods are only extra functionality that only a Client supports such as FlushNode. Someone using the Rust API for Client now has access to all Apache Arrow Flight server functionality through a single API, instead of having to use Client for some things and a manual Apache Arrow Flight client for other things.

After the new Client was implemented, it was considered if it could be used elsewhere in the system since it now supports all the Apache Arrow Flight functionality. An issue (#417) has been created to explore this further.

Copilot

Pull request overview

This pull request extends the ModelarDB Arrow Flight protocol and server/client implementations to support node-oriented operations, specifically listing cluster nodes and retrieving per-node resource/storage-engine metrics.

Changes:

Added protobuf messages and Rust helpers for encoding/decoding cluster node metadata.
Implemented new Arrow Flight actions ListNodes and NodeMetrics on the server (and added integration tests).
Extended modelardb_embedded::Client with node-related APIs (configuration, flush/kill, list nodes, node metrics), plus supporting error handling.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
crates/modelardb_types/src/flight/protocol.proto	Adds `NodeMetadata`, `ClusterNodes`, and `NodeMetrics` protobuf messages.
crates/modelardb_types/src/flight/mod.rs	Adds cluster-node encode/decode helpers and a unit test.
crates/modelardb_server/tests/integration_test.rs	Adds integration tests for `ListNodes`/`NodeMetrics` and updates action-list expectations.
crates/modelardb_server/src/storage/types.rs	Makes `remaining_ingested_memory_in_bytes` available outside tests.
crates/modelardb_server/src/storage/mod.rs	Exposes remaining-memory getters on `StorageEngine` for metrics reporting.
crates/modelardb_server/src/remote/mod.rs	Implements `ListNodes` and `NodeMetrics` actions and updates `ClusterMode` usage.
crates/modelardb_server/src/main.rs	Removes local `ClusterMode` definition and uses `cluster::ClusterMode`.
crates/modelardb_server/src/data_folders.rs	Stores the single-node’s `Node` metadata inside `ClusterMode::SingleNode`.
crates/modelardb_server/src/context.rs	Implements metric collection via `sysinfo` and disk-space identification logic.
crates/modelardb_server/src/configuration.rs	Updates imports/construction to use the new `cluster::ClusterMode`.
crates/modelardb_server/src/cluster.rs	Defines `ClusterMode` and adds `nodes()` helpers for single-node vs cluster.
crates/modelardb_server/Cargo.toml	Adds `sysinfo` dependency for metrics collection.
crates/modelardb_embedded/src/operations/client.rs	Adds node-related `Client` methods and refactors action sending/receiving.
crates/modelardb_embedded/src/error.rs	Adds a dedicated Prost decode error variant.
crates/modelardb_embedded/Cargo.toml	Adds `prost` dependency needed for decoding response protobufs.
Cargo.lock	Updates lockfile for new dependencies (`sysinfo`, `prost`).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

skejserjensen · 2026-08-13T13:26:40Z

+    async fn send_action(
+        &mut self,
+        action_type: &str,
+        body: Vec<u8>,


Maybe make body parameter &[u8] instead of requiring the caller to create a Vec to pass a body. Although I do not know if it make the conversion into the right format for Action more complex and thus make the code less readable.

skejserjensen · 2026-08-13T13:28:53Z

+        Ok(response.into_inner())
+    }
+
+    /// Returns the URL of the cloud node that the node assigns to execute the SQL in `sql`. If the


Is the second node mentioned in the comment not also a cloud node, since we no longer have a manager? If that is the case, should the node not just always return itself if it is not running in a cluster configuration?

skejserjensen · 2026-08-13T13:35:02Z

+#[derive(Clone)]
+pub(crate) enum ClusterMode {
+    SingleNode(Node),
+    MultiNode(Box<Cluster>),


Is Cluster in Box because clippy warns about its size? Otherwise, I do not understand why the Box. It would probably be good to add a comment describing why Box is used.

skejserjensen · 2026-08-13T13:37:43Z

+    /// Return all nodes currently in the cluster. If the nodes could not be retrieved, return
+    /// [`ModelarDbServerError`].
+    pub(crate) async fn nodes(&self) -> Result<Vec<Node>> {
+        self.remote_data_folder


Since we read the cluster nodes from the data folder, can we get into situations where we return nodes to the caller of nodes() that no longer exist? And if so, is this a problem?

skejserjensen · 2026-08-13T14:00:10Z

+        let mut system = System::new();
+
+        // Sample the CPU twice, separated by the minimum update interval, since a single refresh
+        // reads zero.


Unclear what it meant by "...reads zero." as it does not look like system.refresh_cpu_usage() return a value that is used.

skejserjensen · 2026-08-13T14:02:52Z

+        let compressed_reserved_memory_in_bytes =
+            configuration_manager.compressed_reserved_memory_in_bytes();
+
+        let ingested_used_memory_in_bytes = (ingested_reserved_memory_in_bytes as i64


Maybe add a comment describing what this calculation does as it is probably a bit hard to understand for anybody that does not know how the storage engine manages memory.

skejserjensen · 2026-08-13T14:07:15Z

+
+        let maybe_disk = disks
+            .iter()
+            .filter(|disk| location.starts_with(&*disk.mount_point().to_string_lossy()))


I think I understand the filter() as it seems to remove all of the disks that the data folder is not stored in, however, I dod not understand the purpose of the max_by_key() and or_else() calls.

skejserjensen · 2026-08-13T14:08:52Z

+
+        let maybe_disk = disks
+            .iter()
+            .filter(|disk| location.starts_with(&*disk.mount_point().to_string_lossy()))


Why dereference the string and then borrow it right after? Why is this needed?

skejserjensen · 2026-08-13T14:12:51Z

 }

+/// Encode `nodes` into a [`ClusterNodes`](protocol::ClusterNodes) protobuf message and serialize it.
+pub fn encode_and_serialize_cluster_nodes(nodes: Vec<Node>) -> Vec<u8> {


If the function immediately converts the Vec to an iterator, why not just make the function take any type that impl iterator or into iterator, depending on what is required?

CGodiksen added 28 commits August 11, 2026 07:10

Add proto messages for NodeMetadata and ClusterNodes

1581a0d

Add NodeMetrics proto message

9370183

Add node to ClusterMode::SingleNode for self identity

054c01d

Add sysinfo as a dependency to server

6af2554

Add getters for the remaining memory in bytes to storage engine

d4a9657

Add method to get all nodes in the cluster

bdea1a2

Add util method to get local data folder disk space

8f3252d

Add ListNodes action

469b6ba

Add NodeMetrics action

c6f6916

Add new actions to list_actions

0ce9d4e

Use new nodes method in peer_nodes()

b22605e

Separate ListNodes implementation into relevant helper functions

51d55d5

Move ClusterMode from main.rs into cluster.rs

330ef04

Update imports for ClusterMode

1d74d9d

Move collection of metrics out of do_action and into Context

6ad47b6

Remove misleading comment

f19e3b9

Fix integration test after adding new actions

972ebf7

Add prost decode error to modelardb_embedded

8861c26

Add a method to deserialize and extract cluster nodes from protobuf

50e227b

Add test for serializing and deserializing nodes

259e0d9

Add helper methods to run actions on Client

c4ac14b

Refactor action helper so they can be used in modelardb_type and create

8d57405

Add basic node operations to Client struct

c402383

Change order of helper methods for consistency

2e173f0

Add a method that uses get_flight_info to get the cloud query node

1735212

Add integration test for list nodes action

7ddb124

Add integration test for node metrics action

b890074

Simplify subtraction for used disk space

d3a6235

CGodiksen self-assigned this Aug 12, 2026

CGodiksen requested a lite review from Copilot August 12, 2026 11:14

CGodiksen requested review from skejserjensen and a lite review from Copilot and removed request for Copilot August 12, 2026 12:23

Copilot started reviewing on behalf of CGodiksen August 13, 2026 08:12 View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Comment thread crates/modelardb_server/src/context.rs

Comment thread crates/modelardb_embedded/src/operations/client.rs Outdated

Comment thread crates/modelardb_server/tests/integration_test.rs

Add better error handling in retrieve_action_bytes()

33d1bd8

skejserjensen requested changes Aug 13, 2026

View reviewed changes

Navigation Menu

Uh oh!

Add actions to list nodes and retrieve node metrics - #418

Add actions to list nodes and retrieve node metrics#418
CGodiksen wants to merge 29 commits into
mainfrom
dev/node-operations

CGodiksen commented Aug 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skejserjensen Aug 13, 2026

Uh oh!

skejserjensen Aug 13, 2026

Uh oh!

skejserjensen Aug 13, 2026

Uh oh!

skejserjensen Aug 13, 2026

Uh oh!

skejserjensen Aug 13, 2026

Uh oh!

skejserjensen Aug 13, 2026

Uh oh!

skejserjensen Aug 13, 2026

Uh oh!

skejserjensen Aug 13, 2026

Uh oh!

skejserjensen Aug 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

CGodiksen commented Aug 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

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!

3 participants