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/06/24 17:39:10 UTC
[PR] feat(cpp): add functions related to offset, tests related to consumer groups [iggy]
slbotbm opened a new pull request, #3559:
URL: https://github.com/apache/iggy/pull/3559
## Which issue does this PR address?
<!--
We generally require a GitHub issue for all bug fixes and enhancements. Link it with GitHub syntax, keep the line that applies and delete the other:
- `Closes #123` auto-closes the issue when this PR merges (full fix).
- `Relates to #123` links without closing (partial or related work).
-->
Closes #
Relates to #2100
## Rationale
<!--
Why is this change needed? If the issue explains it well, a one-liner is fine.
-->
Additions of more functions and tests in the cpp sdk.
## 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)
-->
- decouples delete_connection from logout_user function.
- adds logout_user, update_stream, store_consume_offset, get_consumer_offset, delete_consumer_offset, and flush_unsaved_buffer functions
- adds tests for join_consumer_group and leave_consumer_group, which were missing previously. Added cleanup related to consumer groups as well.
- All conversion code was moved into type_conversions.rs for ease of finding.
- Also added consumer and producer types for high-level api, and commented-out method signatures for the rest of the remaining functions in lib.rs
## 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. Which tools? codex
2. Scope of usage? code generation, researcg
3. How did you verify the generated code works correctly? tested it, read through it
4. Can you explain every line of the code if asked? yes
If no AI tools were used, write "None" or delete this section.
-->
--
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 offset, tests related to consumer groups [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on PR #3559:
URL: https://github.com/apache/iggy/pull/3559#issuecomment-4835005133
/ready
--
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 offset, tests related to consumer groups [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on code in PR #3559:
URL: https://github.com/apache/iggy/pull/3559#discussion_r3493287424
##########
foreign/cpp/build.rs:
##########
@@ -21,7 +21,8 @@ fn main() {
.compile("iggy-cpp-bridge");
println!("cargo:rerun-if-changed=src/client.rs");
- println!("cargo:rerun-if-changed=src/consumer_group.rs");
+ println!("cargo:rerun-if-changed=src/consumer.rs");
+ println!("cargo:rerun-if-changed=src/producer.rs");
Review Comment:
done
--
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 offset, tests related to consumer groups [iggy]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3559:
URL: https://github.com/apache/iggy/pull/3559#issuecomment-4792050236
<!-- iggy-pr-triage-welcome -->
Thanks for the PR. It is labeled `S-waiting-on-review` and queued for review.
Slash commands (own line, regular comment) move it around the queue:
- `/ready` - back to `S-waiting-on-review` after addressing feedback
- `/author` - flip to `S-waiting-on-author` while you finish changes
- `/request-review @user-or-team` - request a reviewer
See [CONTRIBUTING.md](https://github.com/apache/iggy/blob/master/CONTRIBUTING.md#pr-triage-commands) for details.
--
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 offset, tests related to consumer groups [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on code in PR #3559:
URL: https://github.com/apache/iggy/pull/3559#discussion_r3490713717
##########
foreign/cpp/build.rs:
##########
@@ -21,7 +21,8 @@ fn main() {
.compile("iggy-cpp-bridge");
println!("cargo:rerun-if-changed=src/client.rs");
- println!("cargo:rerun-if-changed=src/consumer_group.rs");
+ println!("cargo:rerun-if-changed=src/consumer.rs");
+ println!("cargo:rerun-if-changed=src/producer.rs");
Review Comment:
the `rerun-if-changed` list is out of sync with this PR. lines 29-30 still point at `src/stream.rs` and `src/topic.rs`, both deleted here. once a build script emits any `rerun-if-changed`, cargo stops watching the whole package and only watches the listed paths - and a listed path that no longer exists is treated as always-dirty, so the script reruns on every `cargo build` and recompiles the cxx C++ bridge each time. confirmed locally: with a missing listed path the build script ran on 3 of 3 no-op builds, vs 1 of 3 when every path exists. fix: drop the `stream.rs` and `topic.rs` lines. separately the new `src/type_conversion.rs` isn't listed - not a correctness issue (cxx codegen only reads `lib.rs`, and the crate recompiles on any `.rs` change regardless) but worth adding for consistency.
##########
foreign/cpp/src/client.rs:
##########
@@ -822,6 +777,144 @@ impl Client {
})
}
+ pub fn store_consumer_offset(
Review Comment:
`store_consumer_offset`, `get_consumer_offset` and `delete_consumer_offset` repeat the same two blocks verbatim - the `consumer_kind` match (`"consumer"` -> `Consumer::new`, `"consumer_group"` -> `Consumer::group`, else error) and the `partition_id == ANY_PARTITION_ID` -> `Option` mapping (a 4th copy of the one in `poll_messages`). worth pulling out a small `resolve_consumer(kind, id)` helper plus an `opt_partition(u32)` helper so the kind strings and the sentinel live in one place. drops ~30 lines and removes the risk of the three copies drifting apart.
--
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 offset, tests related to consumer groups [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on code in PR #3559:
URL: https://github.com/apache/iggy/pull/3559#discussion_r3493313197
##########
foreign/cpp/src/client.rs:
##########
@@ -822,6 +777,144 @@ impl Client {
})
}
+ pub fn store_consumer_offset(
Review Comment:
done
--
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 offset, tests related to consumer groups [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio merged PR #3559:
URL: https://github.com/apache/iggy/pull/3559
--
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