feat(cpp): add functions related to system by slbotbm · Pull Request #3102 · apache/iggy

slbotbm · 2026-04-12T14:28:53Z

Which issue does this PR close?

Works toward completion of #2100

Rationale

This PR adds functions related to the system, and tests for previously untested functions.

What changed?

This PR adds:

The following functions and their tests:
- get_stats
- get_me
- get_client
- get_clients
- ping
- heartbeat_interval
- snapshot

The places labelled TODO will be completed after merging of #3046.

Local Execution

Passed
Pre-commit hooks ran

AI Usage

If AI tools were used, please answer:

Which tools? (e.g., GitHub Copilot, Claude, ChatGPT) codex
Scope of usage? (e.g., autocomplete, generated functions, entire implementation) code generation based on explicit instructions given by me.
How did you verify the generated code works correctly? ran the tests, read through and understood the generated code
Can you explain every line of the code if asked? yes

slbotbm · 2026-04-22T11:19:46Z

This PR is ready for review

github-actions · 2026-05-08T00:54:01Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs.

If you need a review, please ensure CI is green and the PR is rebased on the latest master. Don't hesitate to ping the maintainers - either @core on Discord or by mentioning them directly here on the PR.

Thank you for your contribution!

hubcio · 2026-05-14T12:09:53Z

/ready

slbotbm · 2026-05-14T12:11:18Z

@hubcio i still need to completely address the review for this pr

maybe /author is better?

hubcio · 2026-05-14T12:12:45Z

@slbotbm I know. /ready is me marking the PR ready for review. see CONTRIBUTING.md - i added simple triage bot. this PR was created before the bot, so I'm marking manually all existing PRs.

EDIT: you are right! this should be /author...

hubcio · 2026-05-14T12:13:25Z

/author

slbotbm · 2026-05-16T08:04:46Z

/ready

hubcio · 2026-05-20T13:42:20Z

+        RUNTIME.block_on(async { self.inner.heartbeat_interval().await.as_micros() })
+    }
+
+    pub fn snapshot(


empty snapshot_types vec → server loops zero times in core/server/src/shard/system/snapshot/mod.rs:71 and returns an empty zip with Ok(()). silent no-op. duplicate types collide on the {snapshot_type}.txt filename at line 80, so the second entry quietly overwrites the first. reject empty vec at the ffi boundary and dedup before passing to the sdk.

hubcio · 2026-05-20T13:42:20Z

+/// Internally, each selected value is passed across the Rust FFI as a string.
+/// Values outside the supported set are rejected by the Rust client.
+/// Some Rust-side aliases may also be accepted for compatibility.
+class SnapshotType final : private detail::StringTag<SnapshotType> {


rust calls this SystemSnapshotType (core/common/src/types/snapshot/mod.rs:36) and c# preserves the prefix in foreign/csharp/Iggy_SDK/Enums/SystemSnapshotType.cs. c++ binding drops System. rename to SystemSnapshotType while pre-release to stop the divergence.

hubcio · 2026-05-20T13:42:20Z

+///
+/// Internally, this value is passed across the Rust FFI as a string.
+/// Values outside the supported set are rejected by the Rust client.
+/// Some Rust-side aliases may also be accepted for compatibility.


this "Some Rust-side aliases may also be accepted for compatibility." line is repeated on SnapshotCompression at line 78 and SnapshotType at line 107. only MaxTopicSize::from_str accepts genuine token aliases ("0" ≡ "server_default"). the other three only do s.to_lowercase() then exact-match (core/common/src/types/compression/compression_algorithm.rs:45-51, core/common/src/types/snapshot/mod.rs:129-141, 171-181). drop the alias claim from the three inaccurate docs, keep on MaxTopicSize only.

hubcio · 2026-05-20T13:42:20Z

@@ -67,14 +67,14 @@ impl TryFrom<ffi::Identifier> for RustIdentifier {
 // preserves the C++ ABI used by every test and downstream binding.
 #[allow(clippy::wrong_self_convention)]


follow-up on the earlier rename thread: from_string → set_string and from_numeric → set_numeric landed but the #[allow(clippy::wrong_self_convention)] here is now dead (set_* doesn't trigger that lint) and the comment above about "keeping the names preserves the C++ ABI used by every test and downstream binding" directly contradicts the rename in this same pr. delete both.

hubcio · 2026-05-20T13:42:20Z

+        memory_usage: u64,
+        total_memory: u64,
+        available_memory: u64,
+        run_time: u64,


run_time and start_time are both bare u64 but carry different semantics - client.rs:107-108 fills them as stats.run_time.as_micros() (IggyDuration, micros since boot) and stats.start_time.as_micros() (IggyTimestamp, unix-epoch micros). consumers cannot tell them apart without reading the rust source. rename to run_time_micros and start_time_epoch_micros, or doc-comment the bridge struct.

hubcio · 2026-05-20T13:42:20Z

+        ffi::ClientInfo {
+            client_id: client.client_id,
+            has_user_id,
+            user_id: client.user_id.unwrap_or(0),


follow-up on the earlier unwrap_or(u32::MAX) thread: switching to unwrap_or(0) here and at line 60 trades one collision for another - root user has id 0 (core/common/src/http/users/defaults.rs:26, applied at core/server/src/streaming/users/user.rs:96), so (has_user_id=true, user_id=0) for a root login is indistinguishable from (has_user_id=false, user_id=0) for an unauthenticated session whenever a consumer reads user_id without first checking the bool. wire encoding already uses u32::MAX as the none sentinel (core/server/src/binary/handlers/system/get_me_handler.rs:64, decoder at core/common/src/wire_conversions.rs:258-261). either doc the has_user_id contract on the bridge structs (lib.rs:123-130, 132-140), or align ffi to the wire sentinel with unwrap_or(u32::MAX).

hubcio · 2026-05-20T13:42:20Z

+
+// TODO(slbotbmR): add a test to create some streams, topics, partitions, and segments, send messages, and create
+// consumer groups and verify it.
+TEST(LowLevelE2E_Client, GetStatsReturnsServerStats) {


tests/common/test_helpers.hpp:18 already has a TODO(slbotbm): create fixture for setup/teardown covering the broader cleanup story, but this specific test has two extra fragilities not implicit in that todo: (1) fixed stream/topic/group names (cpp-get-stats-stream-1 etc) - any failing ASSERT_NO_THROW before the inline cleanup at lines 266-272 leaks streams and the next run collides on create_stream; (2) clients_count == empty + 2u (line 260) compares deltas against empty_stats taken seconds earlier on a shared server - any other client connecting in between invalidates the assertion. once the fixture lands, randomize names and switch the count assertions to EXPECT_GE so this test stops being order-dependent. add a TODO here if the fix isn't landing in this pr.

hubcio · 2026-05-20T13:42:20Z

+        EXPECT_FALSE(static_cast<std::string>(empty_stats.os_version).empty());
+        EXPECT_FALSE(static_cast<std::string>(empty_stats.kernel_version).empty());
+        EXPECT_FALSE(static_cast<std::string>(empty_stats.iggy_server_version).empty());
+        EXPECT_TRUE(!empty_stats.has_server_semver || empty_stats.iggy_server_semver > 0u);


!has_server_semver || iggy_server_semver > 0u is unfalsifiable. wire decoder at core/binary_protocol/src/responses/system/get_stats.rs:204 sets Some(raw) iff raw != 0, so has_server_semver=true already implies iggy_server_semver > 0. use EXPECT_EQ(empty_stats.has_server_semver, empty_stats.iggy_server_semver != 0u) or drop the assertion.

hubcio · 2026-05-20T13:42:20Z

+            kernel_version: stats.kernel_version,
+            iggy_server_version: stats.iggy_server_version,
+            has_server_semver,
+            iggy_server_semver: stats.iggy_server_semver.unwrap_or(0),


same unwrap_or(0) shape as the user_id follow-up. wire decoder at core/binary_protocol/src/responses/system/get_stats.rs:204 forbids Some(0) from a conforming server so the collision is unreachable today, but the pattern is fragile - if anyone ever changes the wire format to allow Some(0), this silently degrades to the same shape as None. doc the invariant on the bridge struct, or align to a sentinel that cannot collide.

hubcio · 2026-05-20T13:42:20Z

                    format!(
-                        "Could not get consumer group '{}' for topic '{}' on stream '{}': {error}",
-                        rust_group_id, rust_topic_id, rust_stream_id
+                        "Could not get consumer group '{rust_group_id}' for topic '{rust_topic_id}' on stream '{rust_stream_id}': {error}"


follow-up on the earlier clippy::uninlined_format_args thread: the sweep landed here and at line 627, but missed two callers - join_consumer_group at lines 656-657 and leave_consumer_group at 741-742 still use the old positional {} style with separate args. unify to named captures.

amlel-el-mahrouss · 2026-05-21T15:46:34Z

+template <typename Tag>
+class StringTag {
+  protected:
+    explicit StringTag(std::string value) : value_(std::move(value)) {}
+
+    std::string_view value() const { return value_; }
+
+  private:
+    std::string value_;
+};


StringTag should be marked final and have a destructor to satisfy the C++ ABI.

amlel-el-mahrouss · 2026-05-21T15:49:08Z

+    static SnapshotCompression stored() { return SnapshotCompression("stored"); }
+    /// Use standard deflate compression.
+    static SnapshotCompression deflated() { return SnapshotCompression("deflated"); }
+    /// Use bzip2 for a higher compression ratio at the cost of slower processing.
+    static SnapshotCompression bzip2() { return SnapshotCompression("bzip2"); }
+    /// Use Zstandard for fast compression and decompression.
+    static SnapshotCompression zstd() { return SnapshotCompression("zstd"); }
+    /// Use LZMA for high compression, especially for larger files.
+    static SnapshotCompression lzma() { return SnapshotCompression("lzma"); }
+    /// Use XZ, which is similar to LZMA and often faster to decompress.
+    static SnapshotCompression xz() { return SnapshotCompression("xz"); }
+
+    std::string_view snapshot_compression_value() const { return value(); }


Shouldn't this be in a trait type instead?

amlel-el-mahrouss

Requesting the following changes for:

StringType
SnapshotCompression

See the comments for more info.

slbotbm marked this pull request as draft April 12, 2026 14:29

slbotbm changed the title ~~feata(cpp): add functions related to system, tests for purge_* functions, and consumer group functions (WIP)~~ feat(cpp): add functions related to system, tests for purge_* functions, and consumer group functions (WIP) Apr 12, 2026

slbotbm marked this pull request as ready for review April 22, 2026 11:19

slbotbm changed the title ~~feat(cpp): add functions related to system, tests for purge_* functions, and consumer group functions (WIP)~~ feat(cpp): add functions related to system Apr 25, 2026

hubcio requested changes Apr 29, 2026

View reviewed changes

github-actions Bot added the S-stale Inactive issue or pull request label May 8, 2026

slbotbm marked this pull request as draft May 10, 2026 14:39

slbotbm added 3 commits May 11, 2026 22:05

add system functions

32202b7

partially address review

6ead030

rebase to master

4e2082a

slbotbm force-pushed the cpp-system-functions branch from c6be262 to 4e2082a Compare May 11, 2026 13:23

github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label May 14, 2026

github-actions Bot added S-waiting-on-author PR is waiting on author response and removed S-waiting-on-review PR is waiting on a reviewer labels May 14, 2026

slbotbm added 2 commits May 16, 2026 16:25

add docs + warning to iggy.hpp

77338cc

address testing feedback

87123cd

slbotbm marked this pull request as ready for review May 16, 2026 08:01

github-actions Bot added S-waiting-on-review PR is waiting on a reviewer and removed S-waiting-on-author PR is waiting on author response S-stale Inactive issue or pull request labels May 16, 2026

hubcio requested changes May 20, 2026

View reviewed changes

github-actions Bot added S-waiting-on-author PR is waiting on author response and removed S-waiting-on-review PR is waiting on a reviewer labels May 20, 2026

amlel-el-mahrouss reviewed May 21, 2026

View reviewed changes

amlel-el-mahrouss suggested changes May 21, 2026

View reviewed changes

		@@ -67,14 +67,14 @@ impl TryFrom<ffi::Identifier> for RustIdentifier {
		// preserves the C++ ABI used by every test and downstream binding.
		#[allow(clippy::wrong_self_convention)]

Conversation

slbotbm commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale

What changed?

Local Execution

AI Usage

Uh oh!

slbotbm commented Apr 22, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 8, 2026

Uh oh!

hubcio commented May 14, 2026

Uh oh!

slbotbm commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hubcio commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hubcio commented May 14, 2026

Uh oh!

slbotbm commented May 16, 2026

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!

amlel-el-mahrouss left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

slbotbm commented Apr 12, 2026 •

edited

Loading

slbotbm commented May 14, 2026 •

edited

Loading

hubcio commented May 14, 2026 •

edited

Loading