Allow a log to start at a non-zero offset - #241
Merged
Merged
Conversation
The `initial_offset` config key was only honoured for the acceptor writer type, where it is derived from the writer's offset range. Honouring it for writers too lets a brand new stream start at an offset other than 0. Two consequences are worth calling out. An empty log now resumes from the offset its only segment is named after rather than from the configured value, so a restart before the first write is correct even if the configuration has changed since. And the offset counter is initialised to one before the log's first offset, without which `last_offset + 1 - first_offset` reports a negative message count for an empty log.
There was a problem hiding this comment.
🟡 Changes recommended
Empty replicated logs cannot restart safely after configuration changes, and replication-state queries can crash before the first write.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Enables new streams to begin at a configured non-zero offset and recover that offset from disk.
Changes:
- Applies
initial_offsetto writers, readers, acceptors, and counters. - Recovers empty-log offsets from segment filenames.
- Adds single-node, replication, recovery, reader, and counter tests.
File summaries
| File | Description |
|---|---|
src/osiris.erl |
Adds initial_offset to stream configuration. |
src/osiris_log.erl |
Implements non-zero offset initialization and recovery. |
src/osiris_writer.erl |
Initializes empty-writer commit state. |
test/osiris_SUITE.erl |
Adds end-to-end writer and replication tests. |
test/osiris_log_SUITE.erl |
Adds log-level offset tests. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
An empty writer range carries no offset, so a replica restarting while the
writer is still empty fell back to its own local config for the offset to
resume at. That config can be stale relative to the writer's actual log
(the writer itself always resumes from its segment filename), causing
init_data_reader/2 to reject the replica as out of range and replication
to never restart. osiris_writer:overview/1 now reports the writer's actual
empty-log starting offset alongside the range, and osiris_replica uses it
instead of the replica's own config.
query_replication_state's empty-tail branch also only matched the literal
{0, empty}, so a writer whose empty log starts elsewhere hit a case_clause
as soon as any caller queried it, e.g. rabbit_stream_coordinator:add_replica/2.
Generalised to any {N, empty}.
There was a problem hiding this comment.
🟡 Changes recommended
The changed RPC tuple contract prevents replicas from starting in mixed-version clusters.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
Adding a third element to overview/1's return value broke replication
between mixed-version nodes: it is called over RPC, so during a rolling
upgrade an old replica querying a new writer, or a new replica querying
an old writer, would hit a case_clause on the shape mismatch, for every
stream regardless of whether it uses a non-zero initial offset.
overview/1 goes back to its original {Range, EpochOffsets} shape. The
writer's empty-log starting offset is now fetched through a separate
starting_offset/1 call, made only when the range is empty, with the
replica falling back to 0 if the writer doesn't have the function yet
(an old writer can only ever have an empty log starting at 0).
There was a problem hiding this comment.
🟡 Changes recommended
Replica initialization incorrectly treats every starting-offset RPC failure as a valid zero offset.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
The catch-all also mapped nodedown, timeouts and a dead writer process to offset 0. For a writer whose empty log starts at 100, a replica that hit any of those would initialise its log at 0, have its data reader rejected as out of range and restart, rather than retrying the query. Only the remote undef reply, which identifies a writer that predates starting_offset/1, now implies 0. Every other failure stops the replica the same way a failed overview/1 call already does. The offset query moved into its own function, which meant splitting the tail of handle_continue/2 into start_replica/6. Most of the diff is that body being re-indented; `git diff -w` shows the actual change.
ansd
marked this pull request as ready for review
September 4, 2026 10:33
|
Tick the box to add this pull request to the merge queue (same as
|
ansd
marked this pull request as draft
September 4, 2026 10:43
Reverts the replica-side query added in bb8717b, 389b2a7 and e2ba12f. Every member of a stream is started from the same config, so a replica can read the initial offset from it directly instead of asking the writer for it over RPC. In RabbitMQ that config is built once from the queue's immutable x-stream-initial-offset argument and replicated to all members by the stream coordinator, and a non-zero value is gated behind a feature flag, so no member can be left honouring a different offset. The expectation is now stated on the config key itself. osiris_replica is untouched again, which also removes the RPC wire-compatibility and error-handling problems the query introduced. Kept from those commits is the empty-tail handling in query_replication_state/1, which is an unrelated crash: it only matched {0, empty}, so a writer whose empty log starts elsewhere hit a case_clause as soon as rabbit_stream_coordinator:add_replica/2 queried it.
kjnilsson
self-requested a review
September 4, 2026 12:57
kjnilsson
approved these changes
Sep 4, 2026
ansd
added a commit
to rabbitmq/rabbitmq-server
that referenced
this pull request
Sep 4, 2026
since rabbitmq/osiris#241 was merged into osiris main branch and the initial-offset branch was deleted
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.
The
initial_offsetconfig key was only honoured for the acceptor writer type, where it is derived from the writer's offset range. Honouring it for writers too lets a brand new stream start at an offset other than 0.Two consequences are worth calling out. An empty log now resumes from the offset its only segment is named after rather than from the configured value, so a restart before the first write is correct even if the configuration has changed since. And the offset counter is initialised to one before the log's first offset, without which
last_offset + 1 - first_offsetreports a negative message count for an empty log.