Skip to content

Avoid ftruncate during ledger rollback#7979

Open
achamayou with Copilot wants to merge 11 commits into
mainfrom
copilot/avoid-ftruncates
Open

Avoid ftruncate during ledger rollback#7979
achamayou with Copilot wants to merge 11 commits into
mainfrom
copilot/avoid-ftruncates

Conversation

Copilot AI commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Closes #7837.

Ledger rollback used ftruncate() on the hot path, which is expensive on Azure Files. Committed ledger files still need canonical sizes and contents across nodes, so physical truncation is deferred until chunk completion.

  • Rollback path
    • Replaces immediate physical shrinking with a marker at the logical end of the ledger file.
    • Keeps the marker ahead of subsequent writes, including non-committable writes, so a restart cannot recover stale tail entries.
  • Recovery path
    • Recognises the marker as an intentional logical end rather than malformed ledger data.
    • Preserves the durable incomplete-file transition when replaying existing completed chunks.
  • Completion path
    • Performs the eventual ftruncate() before publishing the positions table offset.
    • Produces canonical completed and committed file bytes.
  • Coverage
    • Covers deferred physical shrinking, graceful and abrupt restart recovery, marker movement after new writes, and byte-for-byte equivalence with a clean committed ledger.

Copilot AI and others added 5 commits June 24, 2026 21:13
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
achamayou and others added 4 commits June 25, 2026 21:24
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@achamayou
achamayou marked this pull request as ready for review July 10, 2026 23:07
@achamayou
achamayou requested a review from a team as a code owner July 10, 2026 23:07
Copilot AI review requested due to automatic review settings July 10, 2026 23:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 addresses #7837 by removing ftruncate() from the ledger rollback hot path (notably expensive on Azure Files) while preserving the invariant that completed/committed ledger chunk files are byte-identical across nodes. It introduces a “logical truncation” marker to prevent recovery from replaying stale tail bytes, and defers the eventual physical truncation to chunk completion time.

Changes:

  • Add a truncation-marker mechanism in asynchost::LedgerFile to represent a logical rollback without immediately shrinking the file.
  • Teach incomplete-ledger recovery to treat the marker as an intentional logical EOF and track deferred physical truncation state.
  • Add a host-ledger unit test covering deferred shrink behavior, crash/restart recovery behavior, and byte-for-byte equivalence with a canonical committed ledger.

Custom instructions used:

  • .github/copilot-instructions.md
  • .github/instructions/reviewing.instructions.md

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/host/ledger.h Implements deferred truncation via a truncation marker during rollback/rewrite, recognizes marker during recovery, and performs physical truncation during completion.
src/host/test/ledger.cpp Adds a regression test validating deferred physical shrink, crash recovery behavior, and equivalence of committed outputs versus a canonical baseline.

Comment thread src/host/ledger.h Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.

Avoid ftruncates

3 participants