You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iggy.apache.org by "hubcio (via GitHub)" <gi...@apache.org> on 2026/02/10 12:59:25 UTC
[PR] refactor(metadata): add typed return values to state handlers [iggy]
hubcio opened a new pull request, #2714:
URL: https://github.com/apache/iggy/pull/2714
mod.rs mixed traits, unsafe left-right infra, and code-gen macros in a single 400-line file, making each concern hard to review independently. StateHandler also returned void, preventing commands from producing results through the left-right layer.
Split into focused modules (traits.rs, left_right.rs, macros.rs) and introduce CommandEnvelope to smuggle typed return values out of Absorb's void interface. StateHandler and Command now carry an Output associated type. WriteCell gains a debug-mode reentrancy guard and LeftRight exposes with_state() for lock-free reads.
--
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] refactor(metadata): add typed return values to state handlers [iggy]
Posted by "numinnex (via GitHub)" <gi...@apache.org>.
numinnex commented on code in PR #2714:
URL: https://github.com/apache/iggy/pull/2714#discussion_r2799102987
##########
core/metadata/src/stm/mod.rs:
##########
@@ -20,76 +20,138 @@ pub mod mux;
pub mod stream;
pub mod user;
+use iggy_common::IggyTimestamp;
use left_right::*;
-use std::cell::UnsafeCell;
+use std::cell::{Cell, UnsafeCell};
use std::sync::Arc;
-pub struct WriteCell<T, O>
+/// Per-command handler for a given state type.
+/// Each command struct implements this for the state it mutates.
+///
+/// `now` is captured once before the command enters `left_right`, ensuring
+/// both copies see the same timestamp (deterministic replay).
+pub trait StateHandler {
+ type State;
+ type Output;
+ fn apply(&self, state: &mut Self::State, now: IggyTimestamp) -> Self::Output;
+}
+
+/// Parses type-erased input into a command. Macro-generated.
+/// Returns `Ok(cmd)` if applicable, `Err(input)` to pass ownership back.
+pub trait Command {
+ type Cmd;
+ type Input;
+ type Output;
+
+ fn parse(input: Self::Input) -> Result<Self::Cmd, Self::Input>;
+}
+
+/// Public interface for state machines.
+/// Returns `Ok(output)` if applicable, `Err(input)` to pass ownership back.
+pub trait State {
+ type Output;
Review Comment:
This output should be generic over the command response type, but type erased.
##########
core/metadata/src/stm/mod.rs:
##########
@@ -20,76 +20,138 @@ pub mod mux;
pub mod stream;
pub mod user;
+use iggy_common::IggyTimestamp;
use left_right::*;
-use std::cell::UnsafeCell;
+use std::cell::{Cell, UnsafeCell};
use std::sync::Arc;
-pub struct WriteCell<T, O>
+/// Per-command handler for a given state type.
+/// Each command struct implements this for the state it mutates.
+///
+/// `now` is captured once before the command enters `left_right`, ensuring
+/// both copies see the same timestamp (deterministic replay).
+pub trait StateHandler {
+ type State;
+ type Output;
+ fn apply(&self, state: &mut Self::State, now: IggyTimestamp) -> Self::Output;
+}
+
+/// Parses type-erased input into a command. Macro-generated.
+/// Returns `Ok(cmd)` if applicable, `Err(input)` to pass ownership back.
+pub trait Command {
+ type Cmd;
+ type Input;
+ type Output;
+
+ fn parse(input: Self::Input) -> Result<Self::Cmd, Self::Input>;
+}
+
+/// Public interface for state machines.
+/// Returns `Ok(output)` if applicable, `Err(input)` to pass ownership back.
+pub trait State {
+ type Output;
+ type Input;
+
+ fn apply(&self, input: Self::Input) -> Result<Self::Output, Self::Input>;
Review Comment:
apply<T> -> Result<Self::Output<T>, Self::Inptu>;
--
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] refactor(metadata): add typed return values to state handlers [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio closed pull request #2714: refactor(metadata): add typed return values to state handlers
URL: https://github.com/apache/iggy/pull/2714
--
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] refactor(metadata): add typed return values to state handlers [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on code in PR #2714:
URL: https://github.com/apache/iggy/pull/2714#discussion_r2811113109
##########
core/metadata/src/stm/mod.rs:
##########
@@ -20,76 +20,138 @@ pub mod mux;
pub mod stream;
pub mod user;
+use iggy_common::IggyTimestamp;
use left_right::*;
-use std::cell::UnsafeCell;
+use std::cell::{Cell, UnsafeCell};
use std::sync::Arc;
-pub struct WriteCell<T, O>
+/// Per-command handler for a given state type.
+/// Each command struct implements this for the state it mutates.
+///
+/// `now` is captured once before the command enters `left_right`, ensuring
+/// both copies see the same timestamp (deterministic replay).
+pub trait StateHandler {
+ type State;
+ type Output;
+ fn apply(&self, state: &mut Self::State, now: IggyTimestamp) -> Self::Output;
+}
+
+/// Parses type-erased input into a command. Macro-generated.
+/// Returns `Ok(cmd)` if applicable, `Err(input)` to pass ownership back.
+pub trait Command {
+ type Cmd;
+ type Input;
+ type Output;
+
+ fn parse(input: Self::Input) -> Result<Self::Cmd, Self::Input>;
+}
+
+/// Public interface for state machines.
+/// Returns `Ok(output)` if applicable, `Err(input)` to pass ownership back.
+pub trait State {
+ type Output;
+ type Input;
+
+ fn apply(&self, input: Self::Input) -> Result<Self::Output, Self::Input>;
Review Comment:
as we previously discussed, it'll stay as is
##########
core/metadata/src/stm/mod.rs:
##########
@@ -20,76 +20,138 @@ pub mod mux;
pub mod stream;
pub mod user;
+use iggy_common::IggyTimestamp;
use left_right::*;
-use std::cell::UnsafeCell;
+use std::cell::{Cell, UnsafeCell};
use std::sync::Arc;
-pub struct WriteCell<T, O>
+/// Per-command handler for a given state type.
+/// Each command struct implements this for the state it mutates.
+///
+/// `now` is captured once before the command enters `left_right`, ensuring
+/// both copies see the same timestamp (deterministic replay).
+pub trait StateHandler {
+ type State;
+ type Output;
+ fn apply(&self, state: &mut Self::State, now: IggyTimestamp) -> Self::Output;
+}
+
+/// Parses type-erased input into a command. Macro-generated.
+/// Returns `Ok(cmd)` if applicable, `Err(input)` to pass ownership back.
+pub trait Command {
+ type Cmd;
+ type Input;
+ type Output;
+
+ fn parse(input: Self::Input) -> Result<Self::Cmd, Self::Input>;
+}
+
+/// Public interface for state machines.
+/// Returns `Ok(output)` if applicable, `Err(input)` to pass ownership back.
+pub trait State {
+ type Output;
Review Comment:
as we previously discussed, it'll stay as is
--
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] refactor(metadata): add typed return values to state handlers [iggy]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #2714:
URL: https://github.com/apache/iggy/pull/2714#issuecomment-3877510337
## [Codecov](https://app.codecov.io/gh/apache/iggy/pull/2714?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `77.61194%` with `45 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 68.77%. Comparing base ([`0aa1f38`](https://app.codecov.io/gh/apache/iggy/commit/0aa1f386449df0ea4fb9d7a2b99b881751cfde61?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`95c11e2`](https://app.codecov.io/gh/apache/iggy/commit/95c11e235b6d60e46d5740051505a7b2a89779dd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/iggy/pull/2714?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [core/metadata/src/stm/macros.rs](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&filepath=core%2Fmetadata%2Fsrc%2Fstm%2Fmacros.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9tZXRhZGF0YS9zcmMvc3RtL21hY3Jvcy5ycw==) | 71.64% | [18 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [core/metadata/src/stm/stream.rs](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&filepath=core%2Fmetadata%2Fsrc%2Fstm%2Fstream.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9tZXRhZGF0YS9zcmMvc3RtL3N0cmVhbS5ycw==) | 14.28% | [12 Missing :warning: ](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [core/metadata/src/stm/user.rs](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&filepath=core%2Fmetadata%2Fsrc%2Fstm%2Fuser.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9tZXRhZGF0YS9zcmMvc3RtL3VzZXIucnM=) | 18.18% | [9 Missing :warning: ](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [core/metadata/src/stm/left\_right.rs](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&filepath=core%2Fmetadata%2Fsrc%2Fstm%2Fleft_right.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9tZXRhZGF0YS9zcmMvc3RtL2xlZnRfcmlnaHQucnM=) | 94.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [core/metadata/src/stm/consumer\_group.rs](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&filepath=core%2Fmetadata%2Fsrc%2Fstm%2Fconsumer_group.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9tZXRhZGF0YS9zcmMvc3RtL2NvbnN1bWVyX2dyb3VwLnJz) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #2714 +/- ##
============================================
+ Coverage 68.62% 68.77% +0.14%
Complexity 637 637
============================================
Files 734 735 +1
Lines 59660 59759 +99
Branches 56073 56172 +99
============================================
+ Hits 40943 41098 +155
+ Misses 16717 16656 -61
- Partials 2000 2005 +5
```
| [Flag](https://app.codecov.io/gh/apache/iggy/pull/2714/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [rust](https://app.codecov.io/gh/apache/iggy/pull/2714/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `69.96% <77.61%> (+0.15%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files with missing lines](https://app.codecov.io/gh/apache/iggy/pull/2714?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [core/metadata/src/stm/mux.rs](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&filepath=core%2Fmetadata%2Fsrc%2Fstm%2Fmux.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9tZXRhZGF0YS9zcmMvc3RtL211eC5ycw==) | `98.63% <100.00%> (+3.89%)` | :arrow_up: |
| [core/metadata/src/stm/consumer\_group.rs](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&filepath=core%2Fmetadata%2Fsrc%2Fstm%2Fconsumer_group.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9tZXRhZGF0YS9zcmMvc3RtL2NvbnN1bWVyX2dyb3VwLnJz) | `0.00% <0.00%> (ø)` | |
| [core/metadata/src/stm/left\_right.rs](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&filepath=core%2Fmetadata%2Fsrc%2Fstm%2Fleft_right.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9tZXRhZGF0YS9zcmMvc3RtL2xlZnRfcmlnaHQucnM=) | `94.00% <94.00%> (ø)` | |
| [core/metadata/src/stm/user.rs](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&filepath=core%2Fmetadata%2Fsrc%2Fstm%2Fuser.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9tZXRhZGF0YS9zcmMvc3RtL3VzZXIucnM=) | `14.16% <18.18%> (+14.16%)` | :arrow_up: |
| [core/metadata/src/stm/stream.rs](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&filepath=core%2Fmetadata%2Fsrc%2Fstm%2Fstream.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9tZXRhZGF0YS9zcmMvc3RtL3N0cmVhbS5ycw==) | `6.66% <14.28%> (+6.66%)` | :arrow_up: |
| [core/metadata/src/stm/macros.rs](https://app.codecov.io/gh/apache/iggy/pull/2714?src=pr&el=tree&filepath=core%2Fmetadata%2Fsrc%2Fstm%2Fmacros.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9tZXRhZGF0YS9zcmMvc3RtL21hY3Jvcy5ycw==) | `71.64% <71.64%> (ø)` | |
... and [5 files with indirect coverage changes](https://app.codecov.io/gh/apache/iggy/pull/2714/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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] refactor(metadata): add typed return values to state handlers [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on PR #2714:
URL: https://github.com/apache/iggy/pull/2714#issuecomment-3908416482
Closing this: we aren't sure yet how to properly implement it yet.
--
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