feat(server): FIFO channels data plane (PUSH/POP/GET_CHAN_LEN) (PR 2) - #298
Merged
Conversation
PUSH appends owner-authored elements to a FIFO channel's log; the logical
queue depth (cursor-to-tail span, not physical entry count) is gated by
`max_persist_messages`, returning `QUEUE_FULL` on overflow. PUSH durability
mirrors `BROADCAST`: synchronous flush+fsync when
`message_flush_interval == 0`, batched otherwise.
POP atomically returns the head element. The handler reads the entry at
`cursor.next_seq`, forces a synchronous log flush when the entry is not yet
durable (`next_seq > last_durable_seq`), advances the cursor sidecar with
an atomic tmp+fsync+rename, and only then writes `POP_ACK`. A crash between
cursor fsync and the socket write loses the element consumer-side; no
element is ever returned twice. `POP_ACK` carries the entry's PUSH
timestamp so consumers can reason about queue-induced latency.
GET_CHAN_LEN reports the underflow-guarded logical depth `log.last_seq -
cursor.next_seq + 1`. Owner-only.
Adds:
- `PUSH`/`PUSH_ACK`/`POP`/`POP_ACK`/`GET_CHAN_LEN`/`CHAN_LEN` protocol
variants with round-trip tests.
- Mailbox commands and handlers on `ChannelManager` with full validation
matrix (`CHANNEL_NOT_FOUND`, `WRONG_TYPE`, `USER_NOT_IN_CHANNEL`,
`FORBIDDEN`, `NOT_ALLOWED`, `QUEUE_EMPTY`, `QUEUE_FULL`,
`POLICY_VIOLATION`, `CURSOR_RECOVERY_REQUIRED`, `NOT_IMPLEMENTED`).
- C2S dispatch wiring plus `push()`, `pop()`, `get_chan_len()` methods on
both compio and tokio clients.
- Metrics under the `narwhal` prefix: `fifo_pushes{result}`,
`fifo_pops{result}` (one label per outcome), `fifo_cursor_fsync_seconds`
histogram.
- 12 integration tests: PUSH/POP roundtrip, QUEUE_EMPTY/QUEUE_FULL via
logical depth, owner-only PUSH, JOIN-required POP, read-ACL denied POP,
GET_CHAN_LEN happy path + non-owner-FORBIDDEN, competing consumers
(each gets unique element), cursor-durable-across-restart (no element
resurfaces), POP force-flushes log when not durable, PUSH durable with
zero flush interval.
Allow `Message::PopAck` through `ConnTx::send_message_with_payload`'s
payload-bearing variant assertion. Update `docs/PROTOCOL.md` with the six
new messages and flip `docs/architecture/channels/fifo-channels.md` to
"Partially Implemented (CLEAR pending PR 3)".
Signed-off-by: Miguel Ángel Ortuño <ortuman@gmail.com>
PUSH/POP/GET_CHAN_LEN) (PR 2)
1. `Inner::roll_segment` now `sync_all()`s the active log before dropping the handle. Without this, any appends since the last `flush()` (including the entry that triggered the roll) sit in the kernel page cache while the segment is sealed; the subsequent `flush()` only fsyncs the new active segment, so `cached_durable_seq` advances past entries that are not actually on disk. Breaks the durability claim made by `PUSH_ACK` and `BROADCAST_ACK` (with `message_flush_interval == 0`) and risks the `cursor.next_seq <= log.last_seq() + 1` invariant FIFO recovery relies on. 2. `push_payload` no longer increments `channel.seq` before `message_log.append` completes. The previous order (peek + bump via `next_seq()`, then append) burned the seq on a transient append failure, leaving a gap between the in-memory counter and the log's tail. `MessageLog::read(from_seq, ...)` returns the first entry with `seq >= from_seq`, NOT strictly at `from_seq`, so the next POP at the burned seq would read the later entry, advance the cursor by one, and re-read the same entry on the next POP: silent duplicate delivery. The fix peeks `channel.seq` without bumping and commits the bump only after `append` returns Ok. 3. `pop_payload` adds a defensive seq match: `PopVisitor` now records the entry's seq, and the handler verifies `captured.seq == cursor_next_seq` before advancing. On mismatch (gap below the tail), log diagnostics and return `INTERNAL_SERVER_ERROR` instead of advancing. (2) prevents the condition from arising in practice; (3) catches future log-corruption regressions of the same shape and keeps the no-duplication contract load-bearing. Signed-off-by: Miguel Ángel Ortuño <ortuman@gmail.com>
The previous wording elided `MessageLog::read`'s parameters with an ellipsis, which the repo's comment style rule disallows on newly added or modified comments. Spell out the parameter names instead (`from_seq, limit, visitor`). Signed-off-by: Miguel Ángel Ortuño <ortuman@gmail.com>
Synthesize the data-plane and transition-slice entries into shorter summaries, and reclassify both as ENHANCEMENT (they extend the existing channel system rather than introducing a wholly new top-level feature). Signed-off-by: Miguel Ángel Ortuño <ortuman@gmail.com>
Match the brevity of surrounding ENHANCEMENT entries. Signed-off-by: Miguel Ángel Ortuño <ortuman@gmail.com>
This was referenced May 13, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Builds the FIFO data plane on top of PR 1's transition slice. Owners now
PUSHelements to a FIFO channel; JOINed read-allowed members atomicallyPOPthem; owners query the queue depth withGET_CHAN_LEN.The load-bearing contract: each
POPadvances and fsyncs the head cursor sidecar before writingPOP_ACK. A crash between cursor fsync and the socket write loses that element consumer-side; no element is ever returned by twoPOP_ACKs.PUSHdurability matchesBROADCAST(synchronous flush whenmessage_flush_interval == 0, batched otherwise). The cursor advance plus a tail-segment retention invariant in the message log keeps the seq counter recoverable across restart.CLEAR(owner-driven drain / recovery) ships in a follow-up PR.What changed
crates/protocol): 6 new variants —PUSH/PUSH_ACK,POP/POP_ACK,GET_CHAN_LEN/CHAN_LEN.PUSHandPOP_ACKare payload-bearing (BROADCAST/MESSAGE framing convention). 7 round-trip tests.crates/server):Command::{Push,Pop,GetChannelLen}and matching handlers with the full error matrix (CHANNEL_NOT_FOUND,WRONG_TYPE,USER_NOT_IN_CHANNEL,FORBIDDEN,NOT_ALLOWED,QUEUE_EMPTY,QUEUE_FULL,POLICY_VIOLATION,CURSOR_RECOVERY_REQUIRED,NOT_IMPLEMENTED).QUEUE_FULLis gated by the logical cursor-to-tail depth, not the physical entry count. AddsPopVisitornext to the existingHistoryVisitorto capture a single entry into a pooled buffer.crates/server/src/c2s/conn.rs): wire the three new messages.crates/client):push(channel, payload),pop(channel) -> (payload, timestamp),get_chan_len(channel) -> u32on both compio and tokio C2S clients.narwhalprefix):fifo_pushes{result},fifo_pops{result}with one label per outcome, andfifo_cursor_fsync_secondshistogram. Skippedfifo_queue_depth(useGET_CHAN_LENinstead).Message::PopAckadded to thesend_message_with_payloadpayload-bearing variant assertion incrates/common.docs/PROTOCOL.mdgains the six wire messages;docs/architecture/channels/fifo-channels.mdstatus flipped to "Partially Implemented (CLEAR pending PR 3)". CHANGELOG entry.Tests added
crates/server/tests/c2s_fifo_channel.rsgrows by 12 tests:QUEUE_EMPTYon POP against an empty FIFOQUEUE_FULLvia logical depth — fill cap, POP one, PUSH succeeds, PUSH again failsFORBIDDENUSER_NOT_IN_CHANNELNOT_ALLOWEDGET_CHAN_LENreflects PUSH/POP activityGET_CHAN_LENfrom non-owner returnsFORBIDDENmessage_flush_interval, POP still completes immediately)message_flush_interval=0— PUSH, shutdown, restart, POP returns the pre-restart payloadTest plan
cargo fmt --allcleancargo clippy --workspace --all-targets -- -D warningscleancargo test --workspacepasses (all 200+ tests, including the 12 new FIFO data-plane tests plus the 11 PR 1 FIFO tests still green)crates/common/src/conn/mod.rs