Skip to content

Initialise first offset - #223

Draft
dcorbacho wants to merge 1 commit into
mainfrom
initialise-first-offset
Draft

Initialise first offset#223
dcorbacho wants to merge 1 commit into
mainfrom
initialise-first-offset

Conversation

@dcorbacho

@dcorbacho dcorbacho commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

When the log starts at a non-zero offset (e.g. after a retry_accept_chunk reset), reflect that in the first_offset counter so that message-count calculations (last_offset + 1 - first_offset) remain correct.

@dcorbacho
dcorbacho requested a review from kjnilsson June 17, 2026 13:49
@dcorbacho
dcorbacho force-pushed the initialise-first-offset branch from 1e85a25 to 58bddce Compare June 17, 2026 13:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to ensure the log’s first_offset counter reflects a non-zero starting offset (e.g., after retry_accept_chunk resets) so downstream calculations that rely on (last_offset + 1 - first_offset) remain accurate.

Changes:

  • Initialize ?C_FIRST_OFFSET to DefaultNextOffset when the log directory has no existing segments (first_and_last_seginfos/1 returns none).
  • Add inline documentation explaining why this initialization is needed.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/osiris_log.erl
Comment on lines +576 to +580
%% When the log starts at a non-zero offset (e.g. after a
%% retry_accept_chunk reset), reflect that in the first_offset counter
%% so that message-count calculations (last_offset + 1 - first_offset)
%% remain correct.
counters:put(Cnt, ?C_FIRST_OFFSET, DefaultNextOffset),
Comment thread src/osiris_log.erl
%% retry_accept_chunk reset), reflect that in the first_offset counter
%% so that message-count calculations (last_offset + 1 - first_offset)
%% remain correct.
counters:put(Cnt, ?C_FIRST_OFFSET, DefaultNextOffset),
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.

3 participants