Backend API changes - backends independent from filesystems #126

arkjedrz · 2025-09-15T11:49:23Z

No description provided.

Copilot

Pull Request Overview

This PR refactors the KVS (Key-Value Storage) library to make backends independent from filesystems. The primary purpose is to redesign the architecture where KVS instances use a pluggable backend system rather than being tightly coupled to file operations.

Key changes include:

Redesigned KVS architecture with a pluggable backend system via trait objects
Moved file operations from KVS core to JsonBackend implementation
Updated builder pattern to use backend-based configuration instead of directory strings

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/rust/rust_kvs/src/lib.rs	Updated module visibility and prelude to expose backend types
src/rust/rust_kvs/src/kvs_backend.rs	Defined new KvsBackend trait with backend operations
src/rust/rust_kvs/src/kvs_builder.rs	Refactored builder to use backends and instance pooling
src/rust/rust_kvs/src/kvs.rs	Simplified KVS struct to delegate operations to backends
src/rust/rust_kvs/src/json_backend.rs	Moved file operations from KVS core to JsonBackend
src/rust/rust_kvs/src/kvs_api.rs	Updated API to remove file path methods and static functions
tests/rust_test_scenarios/src/main.rs	Commented out test code (temporary during refactoring)
src/rust/rust_kvs_tool/src/kvs_tool.rs	Updated tool to work with new backend system
examples/*	Updated examples to use new backend-based API

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

tests/rust_test_scenarios/src/cit/persistency.rs

github-actions · 2025-09-15T11:53:13Z

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //:license-check

Status: ⚠️ Needs Review

Click to expand output

[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.3.0) and connecting to it...
INFO: Invocation ID: c392d111-f897-4bf2-a8e5-e27ce4e3d445
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
DEBUG: Rule 'rust_qnx8_toolchain+' indicated that a canonical reproducible form can be obtained by modifying arguments integrity = "sha256-oEubHgeZDdT0svMmBKJx7c3/2TdSI/vfwRUyDn+TPGA="
DEBUG: Repository rust_qnx8_toolchain+ instantiated at:
  <builtin>: in <toplevel>
Repository rule http_archive defined at:
  /home/runner/.bazel/external/bazel_tools/tools/build_defs/repo/http.bzl:394:31: in <toplevel>
Computing main repo mapping: 
Computing main repo mapping: 
DEBUG: Rule 'rules_boost+' indicated that a canonical reproducible form can be obtained by modifying arguments integrity = "sha256-iKHWUIBrUN/fFOCltkTmHfDcKw0h4ZVmP7NVSoc8zBs="
DEBUG: Repository rules_boost+ instantiated at:
  <builtin>: in <toplevel>
Repository rule http_archive defined at:
  /home/runner/.bazel/external/bazel_tools/tools/build_defs/repo/http.bzl:394:31: in <toplevel>
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
WARNING: For repository 'rules_cc', the root module requires module version [email protected], but got [email protected] in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'googletest', the root module requires module version [email protected], but got [email protected] in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'platforms', the root module requires module version [email protected], but got [email protected] in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
Computing main repo mapping: 
Loading: 
Loading: 2 packages loaded
Loading: 2 packages loaded
    currently loading: 
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)

Analyzing: target //:license-check (78 packages loaded, 9 targets configured)

Analyzing: target //:license-check (101 packages loaded, 150 targets configured)

Analyzing: target //:license-check (141 packages loaded, 2523 targets configured)

Analyzing: target //:license-check (143 packages loaded, 2564 targets configured)

Analyzing: target //:license-check (146 packages loaded, 4571 targets configured)

Analyzing: target //:license-check (146 packages loaded, 4571 targets configured)

DEBUG: Rule 'toolchains_llvm++llvm+llvm_toolchain_llvm' indicated that a canonical reproducible form can be obtained by modifying arguments _action_listener = <unknown object com.google.devtools.build.lib.packages.Attribute$LabelListLateBoundDefault>, _config_dependencies = [], _configure = False, _environ = [], _original_name = "llvm_toolchain_llvm"
DEBUG: Repository toolchains_llvm++llvm+llvm_toolchain_llvm instantiated at:
  <builtin>: in <toplevel>
Repository rule llvm defined at:
  /home/runner/.bazel/external/toolchains_llvm+/toolchain/rules.bzl:27:23: in <toplevel>
Analyzing: target //:license-check (146 packages loaded, 4571 targets configured)

INFO: Analyzed target //:license-check (147 packages loaded, 6506 targets configured).
[8 / 14] Creating runfiles tree bazel-out/k8-opt-exec-ST-d57f47055a04/bin/external/score_dash_license_checker+/tool/formatters/dash_format_converter.runfiles [for tool]; 0s local ... (2 actions, 1 running)
INFO: From Generating Dash formatted dependency file ...:
INFO: Successfully converted 65 packages from Cargo.lock to bazel-out/k8-fastbuild/bin/formatted.txt
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 207.408s, Critical Path: 0.30s
INFO: 14 processes: 5 disk cache hit, 9 internal.
INFO: Build completed successfully, 14 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

github-actions · 2025-09-15T11:55:34Z

The created documentation from the pull request is available at: docu-html

Copilot

Pull Request Overview

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

Comments suppressed due to low confidence (2)

src/rust/rust_kvs/src/kvs_builder.rs:1

Parameter _kvs has been renamed to kvs but the leading underscore suggests it was intended to indicate an unused parameter. Since the parameter is now used on line 303, the underscore should be removed from the parameter name.

// Copyright (c) 2025 Contributors to the Eclipse Foundation

src/rust/rust_kvs/src/kvs_builder.rs:1

The constant KVS_MAX_SNAPSHOTS is no longer defined in this module after the refactor. This should use kvs.snapshot_max_count() instead to get the maximum snapshot count from the KVS instance.

// Copyright (c) 2025 Contributors to the Eclipse Foundation

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/rust/rust_kvs/src/json_backend.rs

tests/python_test_cases/tests/test_cit_snapshots.py

src/rust/rust_kvs/src/json_backend.rs

Copilot

Pull Request Overview

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

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/rust/rust_kvs/src/json_backend.rs

Copilot · 2025-09-19T12:52:38Z

src/rust/rust_kvs/src/kvs.rs

    fn flush(&self) -> Result<(), ErrorCode> {
+        if self.snapshot_max_count() == 0 {
+            eprintln!("warn: snapshot_max_count == 0, flush ignored");
+            return Ok(());


[nitpick] Silently turning flush into a no-op when snapshot_max_count == 0 may surprise API users: data mutations are never persisted but no error is surfaced. Consider (a) documenting this explicitly, (b) returning a distinct ErrorCode, or (c) still persisting a non-rotated 'current' state even when rotation is disabled.

Suggested change

fn flush(&self) -> Result<(), ErrorCode> {

if self.snapshot_max_count() == 0 {

eprintln!("warn: snapshot_max_count == 0, flush ignored");

return Ok(());

/// * `ErrorCode::SnapshotsDisabled`: Snapshots are disabled (snapshot_max_count == 0)

fn flush(&self) -> Result<(), ErrorCode> {

if self.snapshot_max_count() == 0 {

eprintln!("warn: snapshot_max_count == 0, flush ignored");

return Err(ErrorCode::SnapshotsDisabled);

Copilot · 2025-09-19T12:52:38Z

src/rust/rust_kvs/src/kvs_backend.rs

+/// KVS backend interface.
+pub trait KvsBackend: DynEq + Sync + Send {
+    /// Load KVS content.
+    fn load_kvs(
+        &self,
        instance_id: InstanceId,
        snapshot_id: SnapshotId,
-    ) -> PathBuf;
+    ) -> Result<KvsMap, ErrorCode>;

-    /// Get hash file name.
-    fn hash_file_name(instance_id: InstanceId, snapshot_id: SnapshotId) -> String;
+    /// Load default values.
+    fn load_defaults(&self, instance_id: InstanceId) -> Result<KvsMap, ErrorCode>;

-    /// Get hash file path in working directory.
-    fn hash_file_path(
-        working_dir: &Path,
-        instance_id: InstanceId,
-        snapshot_id: SnapshotId,
-    ) -> PathBuf;
+    /// Flush KvsMap to persistent storage.
+    /// Snapshots are rotated and current state is stored as first (0).
+    fn flush(&self, instance_id: InstanceId, kvs_map: &KvsMap) -> Result<(), ErrorCode>;
+
+    /// Count available snapshots.
+    fn snapshot_count(&self, instance_id: InstanceId) -> usize;

-    /// Get defaults file name.
-    fn defaults_file_name(instance_id: InstanceId) -> String;
+    /// Max number of snapshots.
+    fn snapshot_max_count(&self) -> usize;

-    /// Get defaults file path in working directory.
-    fn defaults_file_path(working_dir: &Path, instance_id: InstanceId) -> PathBuf;
+    /// Restore snapshot with given ID.
+    fn snapshot_restore(
+        &self,
+        instance_id: InstanceId,
+        snapshot_id: SnapshotId,
+    ) -> Result<KvsMap, ErrorCode>;
 }


Public trait lacks documentation about critical behavioral contracts: (a) whether snapshot_id == 0 is always invalid for restore, (b) expected error codes for missing or partially missing snapshot/hash files, (c) flush semantics when snapshot_max_count is 0, and (d) concurrency expectations (atomicity / ordering). Adding these clarifications will make it easier to implement alternative backends correctly.

- New `KvsBackend` API - independent from filesystem. - Make `JsonBackend` explicitly default, allow others. - Add `JsonBackendBuilder`. - This is required to allow for defaults override without setters. - E.g., `snapshot_max_count` can be set, but shouldn't change after init. - Update tests.

arkjedrz · 2025-10-19T16:18:19Z

Merged changes with other PR, moved here #150

arkjedrz requested review from Copilot and vinodreddy-g September 15, 2025 11:49

Copilot AI reviewed Sep 15, 2025

View reviewed changes

tests/rust_test_scenarios/src/cit/persistency.rs Outdated Show resolved Hide resolved

arkjedrz force-pushed the arkjedrz_change-backend-api branch 4 times, most recently from 36fe007 to 876ca7c Compare September 16, 2025 08:11

arkjedrz requested a review from Copilot September 16, 2025 08:13

Copilot AI reviewed Sep 16, 2025

View reviewed changes

src/rust/rust_kvs/src/json_backend.rs Outdated Show resolved Hide resolved

tests/python_test_cases/tests/test_cit_snapshots.py Show resolved Hide resolved

arkjedrz requested a review from PiotrKorkus September 16, 2025 08:30

PiotrKorkus reviewed Sep 16, 2025

View reviewed changes

src/rust/rust_kvs/src/json_backend.rs Outdated Show resolved Hide resolved

src/rust/rust_kvs/src/json_backend.rs Outdated Show resolved Hide resolved

arkjedrz force-pushed the arkjedrz_change-backend-api branch 2 times, most recently from a195eb0 to 70b1b04 Compare September 17, 2025 13:38

arkjedrz self-assigned this Sep 17, 2025

arkjedrz force-pushed the arkjedrz_change-backend-api branch 2 times, most recently from 1a045f1 to 45f46ff Compare September 19, 2025 12:49

arkjedrz requested review from PiotrKorkus and Copilot September 19, 2025 12:49

arkjedrz marked this pull request as ready for review September 19, 2025 12:49

Copilot AI reviewed Sep 19, 2025

View reviewed changes

arkjedrz force-pushed the arkjedrz_change-backend-api branch from 45f46ff to 90bd149 Compare September 29, 2025 13:50

vbogdanb approved these changes Oct 2, 2025

View reviewed changes

arkjedrz force-pushed the arkjedrz_change-backend-api branch 2 times, most recently from b6d018d to 4030295 Compare October 9, 2025 12:55

arkjedrz force-pushed the arkjedrz_change-backend-api branch from 4030295 to 3ffbfaf Compare October 16, 2025 08:32

arkjedrz force-pushed the arkjedrz_change-backend-api branch from 3ffbfaf to 3bdac66 Compare October 19, 2025 16:02

arkjedrz closed this Oct 19, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Backend API changes - backends independent from filesystems #126

Backend API changes - backends independent from filesystems #126

Uh oh!

arkjedrz commented Sep 15, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

github-actions bot commented Sep 15, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Sep 15, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Sep 19, 2025

Uh oh!

Copilot AI Sep 19, 2025

Uh oh!

arkjedrz commented Oct 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Backend API changes - backends independent from filesystems #126

Backend API changes - backends independent from filesystems #126

Uh oh!

Conversation

arkjedrz commented Sep 15, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

github-actions bot commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

License Check Results

Uh oh!

github-actions bot commented Sep 15, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

arkjedrz commented Oct 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Sep 15, 2025 •

edited

Loading