You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iggy.apache.org by "slbotbm (via GitHub)" <gi...@apache.org> on 2026/03/20 14:01:04 UTC
[PR] feat(cpp): Add functions related to consumer groups (WIP) [iggy]
slbotbm opened a new pull request, #2988:
URL: https://github.com/apache/iggy/pull/2988
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Works towards completion of #2763
## Rationale
<!--
Why is this change needed? If the issue explains it well, a one-liner is fine.
-->
Additions to the cpp bindings to make it feature complete.
## What changed?
<!--
2-4 sentences. Problem first (before), then solution (after).
GOOD:
"Messages were unavailable when background message_saver committed the
journal and started async disk I/O before completion. Polling during
this window found neither journal nor disk data.
The fix freezes journal batches in the in-flight buffer before async persist."
GOOD:
"When many small messages accumulate in the journal, the flush passes
thousands of IO vectors to writev(), exceeding IOV_MAX (1024 on Linux)."
BAD:
- Walls of text
- "This PR adds..." (we can see the diff)
-->
Add consumer-group related functions. Also adds commented out purge_topic and purge_stream but those require the ability to actually send messages to the server, which is not yet implemented. Will be added once they can be tested. Also adds create_partition and delete_partitions.
## Local Execution
- Passed
- Pre-commit hooks ran
<!--
You must run your code locally before submitting.
"Relying on CI" is not acceptable - PRs from authors who haven't run the code will be closed.
Did you have `prek` installed? It runs automatically on commit and covers all project languages. See [CONTRIBUTING.md](https://github.com/apache/iggy/blob/master/CONTRIBUTING.md).
-->
## AI Usage
If AI tools were used, please answer:
1. codex
2. code generation
3. manual planning and inspection, local execution
4. Can you explain every line of the code if asked? Yes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): Add functions related to consumer groups [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on code in PR #2988:
URL: https://github.com/apache/iggy/pull/2988#discussion_r2969582495
##########
foreign/cpp/src/lib.rs:
##########
@@ -81,6 +97,49 @@ mod ffi {
message_expiry_value: u64,
max_topic_size: String,
) -> Result<()>;
+ // fn purge_topic(&self, stream_id: Identifier, topic_id: Identifier) -> Result<()>;
+ fn create_partitions(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ partitions_count: u32,
+ ) -> Result<()>;
+ fn delete_partitions(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ partitions_count: u32,
+ ) -> Result<()>;
+ fn create_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ name: String,
+ ) -> Result<ConsumerGroupDetails>;
+ fn get_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<ConsumerGroupDetails>;
+ fn join_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<()>;
+ fn leave_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<()>;
+ fn delete_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<()>;
Review Comment:
Is it something like:
```rust
type ConsumerGroup
impl ConsumerGroup {
def create_consumer_group()
def join_consumer_group()
....
}
```
?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): Add functions related to consumer groups [iggy]
Posted by "amlel-el-mahrouss (via GitHub)" <gi...@apache.org>.
amlel-el-mahrouss commented on code in PR #2988:
URL: https://github.com/apache/iggy/pull/2988#discussion_r2969536167
##########
foreign/cpp/src/lib.rs:
##########
@@ -81,6 +97,49 @@ mod ffi {
message_expiry_value: u64,
max_topic_size: String,
) -> Result<()>;
+ // fn purge_topic(&self, stream_id: Identifier, topic_id: Identifier) -> Result<()>;
+ fn create_partitions(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ partitions_count: u32,
+ ) -> Result<()>;
+ fn delete_partitions(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ partitions_count: u32,
+ ) -> Result<()>;
+ fn create_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ name: String,
+ ) -> Result<ConsumerGroupDetails>;
+ fn get_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<ConsumerGroupDetails>;
+ fn join_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<()>;
+ fn leave_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<()>;
+ fn delete_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<()>;
Review Comment:
This could be wrapped in a `impl Group` right?
##########
foreign/cpp/src/client.rs:
##########
@@ -53,22 +53,22 @@ pub fn new_connection(connection_string: String) -> Result<*mut Client, String>
}
impl Client {
- pub fn connect(&self) -> Result<(), String> {
+ pub fn login_user(&self, username: String, password: String) -> Result<(), String> {
Review Comment:
why do you take `self` but pass `username` and `password` after independently?
##########
foreign/cpp/src/topic.rs:
##########
@@ -25,9 +25,9 @@ impl From<RustTopic> for ffi::Topic {
created_at: topic.created_at.as_micros(),
name: topic.name,
size_bytes: topic.size.as_bytes_u64(),
- message_expiry: topic.message_expiry.into(),
+ message_expiry: u64::from(topic.message_expiry),
Review Comment:
What if it's ill formed data?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): Add functions related to consumer groups [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on code in PR #2988:
URL: https://github.com/apache/iggy/pull/2988#discussion_r2969579504
##########
foreign/cpp/src/lib.rs:
##########
@@ -81,6 +97,49 @@ mod ffi {
message_expiry_value: u64,
max_topic_size: String,
) -> Result<()>;
+ // fn purge_topic(&self, stream_id: Identifier, topic_id: Identifier) -> Result<()>;
+ fn create_partitions(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ partitions_count: u32,
+ ) -> Result<()>;
+ fn delete_partitions(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ partitions_count: u32,
+ ) -> Result<()>;
+ fn create_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ name: String,
+ ) -> Result<ConsumerGroupDetails>;
+ fn get_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<ConsumerGroupDetails>;
+ fn join_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<()>;
+ fn leave_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<()>;
+ fn delete_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<()>;
Review Comment:
What type of implementation are you thinking of? Could you please give a concrete example?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): Add functions related to consumer groups [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio merged PR #2988:
URL: https://github.com/apache/iggy/pull/2988
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): Add functions related to consumer groups [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on PR #2988:
URL: https://github.com/apache/iggy/pull/2988#issuecomment-4103238431
This PR is ready for review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): Add functions related to consumer groups [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on code in PR #2988:
URL: https://github.com/apache/iggy/pull/2988#discussion_r2969578095
##########
foreign/cpp/src/client.rs:
##########
@@ -53,22 +53,22 @@ pub fn new_connection(connection_string: String) -> Result<*mut Client, String>
}
impl Client {
- pub fn connect(&self) -> Result<(), String> {
+ pub fn login_user(&self, username: String, password: String) -> Result<(), String> {
Review Comment:
I did not understand the question. I take username and password from the user to login them in. &self signifies that this method belongs to the Client type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): Add functions related to consumer groups [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on code in PR #2988:
URL: https://github.com/apache/iggy/pull/2988#discussion_r2969541987
##########
foreign/cpp/src/topic.rs:
##########
@@ -25,9 +25,9 @@ impl From<RustTopic> for ffi::Topic {
created_at: topic.created_at.as_micros(),
name: topic.name,
size_bytes: topic.size.as_bytes_u64(),
- message_expiry: topic.message_expiry.into(),
+ message_expiry: u64::from(topic.message_expiry),
Review Comment:
u64::from()/into() are infallible
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): Add functions related to consumer groups [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on PR #2988:
URL: https://github.com/apache/iggy/pull/2988#issuecomment-4162375664
@amlel-el-mahrouss thanks for the comment. utf-16 is not implicitly supported by the rust sdk, which means that I would need to create the encoding/decoding layer for it, which is not something I am inclined to do, and thus it is not supported currently.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): Add functions related to consumer groups [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on PR #2988:
URL: https://github.com/apache/iggy/pull/2988#issuecomment-4153230259
@amlel-el-mahrouss @seokjin0414 mind checking this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): Add functions related to consumer groups [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on PR #2988:
URL: https://github.com/apache/iggy/pull/2988#issuecomment-4119949743
Converting this to a draft since I would like to think a little more deeply about the API we will be exposing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): Add functions related to consumer groups [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on PR #2988:
URL: https://github.com/apache/iggy/pull/2988#issuecomment-4141230717
This PR is ready for review. I was unsure of how to expose the consumer interface to the user, and decided that an additional opaque consumer type will be necessary (to be done in a future PR). The client can now only create or delete consumer groups, but cannot join or read from them. The consumers will be able to join and leave the groups, and also read messages. The lifetimes and safety guarantees of this additional type will still need to be thought through, though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): Add functions related to consumer groups [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on code in PR #2988:
URL: https://github.com/apache/iggy/pull/2988#discussion_r2969608747
##########
foreign/cpp/src/lib.rs:
##########
@@ -81,6 +97,49 @@ mod ffi {
message_expiry_value: u64,
max_topic_size: String,
) -> Result<()>;
+ // fn purge_topic(&self, stream_id: Identifier, topic_id: Identifier) -> Result<()>;
+ fn create_partitions(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ partitions_count: u32,
+ ) -> Result<()>;
+ fn delete_partitions(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ partitions_count: u32,
+ ) -> Result<()>;
+ fn create_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ name: String,
+ ) -> Result<ConsumerGroupDetails>;
+ fn get_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<ConsumerGroupDetails>;
+ fn join_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<()>;
+ fn leave_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<()>;
+ fn delete_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<()>;
Review Comment:
creating a `ConsumerGroup` opaque type would force the end user to handle the clean up for it. I would like to keep the surface area for making a mistake as small as possible, that is why i did not create another opaque type and a implementation based on it. Do you think there are any advantages to creating a `ConsumerGroup` type and its related functions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): Add functions related to consumer groups [iggy]
Posted by "amlel-el-mahrouss (via GitHub)" <gi...@apache.org>.
amlel-el-mahrouss commented on code in PR #2988:
URL: https://github.com/apache/iggy/pull/2988#discussion_r2969598185
##########
foreign/cpp/src/lib.rs:
##########
@@ -81,6 +97,49 @@ mod ffi {
message_expiry_value: u64,
max_topic_size: String,
) -> Result<()>;
+ // fn purge_topic(&self, stream_id: Identifier, topic_id: Identifier) -> Result<()>;
+ fn create_partitions(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ partitions_count: u32,
+ ) -> Result<()>;
+ fn delete_partitions(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ partitions_count: u32,
+ ) -> Result<()>;
+ fn create_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ name: String,
+ ) -> Result<ConsumerGroupDetails>;
+ fn get_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<ConsumerGroupDetails>;
+ fn join_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<()>;
+ fn leave_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<()>;
+ fn delete_consumer_group(
+ &self,
+ stream_id: Identifier,
+ topic_id: Identifier,
+ group_id: Identifier,
+ ) -> Result<()>;
Review Comment:
Yes exactly this kind of type.
##########
foreign/cpp/src/topic.rs:
##########
@@ -25,9 +25,9 @@ impl From<RustTopic> for ffi::Topic {
created_at: topic.created_at.as_micros(),
name: topic.name,
size_bytes: topic.size.as_bytes_u64(),
- message_expiry: topic.message_expiry.into(),
+ message_expiry: u64::from(topic.message_expiry),
Review Comment:
Got it thx
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cpp): Add functions related to consumer groups [iggy]
Posted by "amlel-el-mahrouss (via GitHub)" <gi...@apache.org>.
amlel-el-mahrouss commented on code in PR #2988:
URL: https://github.com/apache/iggy/pull/2988#discussion_r2969599039
##########
foreign/cpp/src/client.rs:
##########
@@ -53,22 +53,22 @@ pub fn new_connection(connection_string: String) -> Result<*mut Client, String>
}
impl Client {
- pub fn connect(&self) -> Result<(), String> {
+ pub fn login_user(&self, username: String, password: String) -> Result<(), String> {
Review Comment:
Nevermind, i dont think my question was a good one anyways, putting username and password in the Client type would be not a good design decision.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org