Skip to content

[Rust] Add mux acknowledgment callback bridge#457

Merged
danilonajkov-db merged 1 commit into
mainfrom
issue-423-mux-callback
Jul 15, 2026
Merged

[Rust] Add mux acknowledgment callback bridge#457
danilonajkov-db merged 1 commit into
mainfrom
issue-423-mux-callback

Conversation

@danilonajkov-db

@danilonajkov-db danilonajkov-db commented Jun 30, 2026

Copy link
Copy Markdown
Member

What changes are proposed in this pull request?

This PR implements the acknowledgment callback bridge required by multiplexed streams.

  • Generalize AckCallback over its identifier type while preserving OffsetId as the default, so existing individual-stream callback implementations remain unchanged.
  • Add a crate-private multiplexed callback adapter that captures one sub-stream index.
  • Convert each acknowledged stream-local OffsetId into the MessageId exposed by MultiplexedStream.
  • Forward both successful acknowledgments and errors through AckCallback<MessageId>.
  • Validate that callback adapters are only created for supported sub-stream indices.

The multiplexed stream builder is being added separately in #433. Its only callback-specific responsibility will be to install one adapter per sub-stream:

let callback = multiplexed_ack_callback(stream_index, callback.clone());
grpc_config.ack_callback = Some(callback);

This gives multiplexed streams the same user-facing callback API as individual streams, except the identifier is MessageId instead of OffsetId:

impl AckCallback<MessageId> for MyCallback {
    fn on_ack(&self, message_id: MessageId) {
        println!("Acknowledged {message_id}");
    }

    fn on_error(&self, message_id: MessageId, error_message: &str) {
        eprintln!("Error for {message_id}: {error_message}");
    }
}

Each adapter already knows which sub-stream it belongs to, so callback delivery requires no lookup, routing table, or locking. It only packs the captured stream index and acknowledged offset into a MessageId before invoking the user callback.

How is this tested?

  • Verifies callbacks from different sub-streams produce different MessageId values for the same stream-local offset.
  • Verifies errors preserve both the correct MessageId and error message.
  • Verifies invalid sub-stream indices are rejected.
  • cargo test -p databricks-zerobus-ingest-sdk
  • cargo clippy -p databricks-zerobus-ingest-sdk --all-targets -- -D warnings
  • cargo fmt --all --check

@danilonajkov-db

danilonajkov-db commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

@elenagaljak-db's comments from previous PR
image

@danilonajkov-db danilonajkov-db changed the title [Rust] Add mux ack callback bridge [Rust] Add mux ack callback Jul 13, 2026
@danilonajkov-db danilonajkov-db changed the title [Rust] Add mux ack callback [Rust] Generalize acknowledgment callback IDs Jul 13, 2026
@danilonajkov-db danilonajkov-db changed the title [Rust] Generalize acknowledgment callback IDs [Rust] Add mux acknowledgment callback bridge Jul 13, 2026
@danilonajkov-db

Copy link
Copy Markdown
Member Author

@elenagaljak-db this should be much cleaner now, we have the adapter/bridge to translate the message_id in mux streams to the offset id in individual streams and made the ack callback IDs generic (so we can reuse the same one for both mux and normal streams)

@danilonajkov-db danilonajkov-db marked this pull request as ready for review July 13, 2026 10:09

@teodordelibasic-db teodordelibasic-db left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread rust/sdk/src/callbacks.rs
fn on_error(&self, id: Id, error_message: &str);
}

#[allow(dead_code)]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I don't think this is needed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ill remove it anyway when I integrate this in the stream builder

Signed-off-by: danilo-najkov-db <danilo.najkov@databricks.com>
@danilonajkov-db danilonajkov-db force-pushed the issue-423-mux-callback branch from 27eab9c to ffaeabc Compare July 15, 2026 12:12
@danilonajkov-db danilonajkov-db added this pull request to the merge queue Jul 15, 2026
Merged via the queue into main with commit 280ad33 Jul 15, 2026
39 checks passed
@danilonajkov-db danilonajkov-db deleted the issue-423-mux-callback branch July 15, 2026 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants