From c0edc56ef785f8079dffca3e1ce3bdd59087aa27 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Jun 2026 21:05:24 +0000 Subject: [PATCH 1/6] Initial plan From 76f4a634bb34952a045b35b23cc2685a5501582b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Jun 2026 21:13:31 +0000 Subject: [PATCH 2/6] Defer ledger file truncation until completion Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com> --- src/host/ledger.h | 99 ++++++++++++++++++++++++++++++---------- src/host/test/ledger.cpp | 45 ++++++++++++++++++ 2 files changed, 121 insertions(+), 23 deletions(-) diff --git a/src/host/ledger.h b/src/host/ledger.h index 432f6d1a534..f5db56991fd 100644 --- a/src/host/ledger.h +++ b/src/host/ledger.h @@ -104,6 +104,75 @@ namespace asynchost // checked against the existing ones, until a divergence is found. bool from_existing_file = false; + [[nodiscard]] size_t get_physical_file_size() + { + if (fseeko(file, 0, SEEK_END) != 0) + { + throw std::logic_error(fmt::format( + "Failed to seek to end of ledger file {}: {}", + file_name, + ccf::nonstd::strerror(errno))); + } + + const auto physical_size = ftello(file); + if (physical_size < 0) + { + throw std::logic_error(fmt::format( + "Failed to read size of ledger file {}: {}", + file_name, + ccf::nonstd::strerror(errno))); + } + + return static_cast(physical_size); + } + + void truncate_physical_file(size_t size) + { + const auto physical_size = get_physical_file_size(); + if (physical_size == size) + { + return; + } + + TimeBoundLogger log_if_slow( + fmt::format("Truncating ledger file - ftruncate({})", file_name)); + if (ftruncate(fileno(file), size) != 0) + { + throw std::logic_error(fmt::format( + "Failed to truncate ledger: {}", ccf::nonstd::strerror(errno))); + } + } + + void write_truncation_marker(size_t physical_size) + { + if (physical_size < total_len + ccf::kv::serialised_entry_header_size) + { + return; + } + + if (fseeko(file, total_len, SEEK_SET) != 0) + { + throw std::logic_error(fmt::format( + "Failed to seek to truncation marker in ledger file {}: {}", + file_name, + ccf::nonstd::strerror(errno))); + } + + ccf::kv::SerialisedEntryHeader marker; + marker.set_size( + (1ULL << ccf::kv::SerialisedEntryHeader::BITS_FOR_SIZE) - 1); + + TimeBoundLogger log_if_slow(fmt::format( + "Writing ledger truncation marker - fwrite({})", file_name)); + if (fwrite(&marker, sizeof(marker), 1, file) != 1) + { + throw std::logic_error(fmt::format( + "Failed to write truncation marker to ledger file {}: {}", + file_name, + ccf::nonstd::strerror(errno))); + } + } + public: // Used when creating a new (empty) ledger file LedgerFile(const fs::path& dir, size_t start_idx, bool recovery = false) : @@ -564,6 +633,9 @@ namespace asynchost positions.resize(idx - start_idx + 1); } + const auto physical_size = get_physical_file_size(); + write_truncation_marker(physical_size); + { TimeBoundLogger log_if_slow( fmt::format("Flushing truncated ledger - fflush({})", file_name)); @@ -574,16 +646,6 @@ namespace asynchost } } - { - TimeBoundLogger log_if_slow( - fmt::format("Truncating ledger file - ftruncate({})", file_name)); - if (ftruncate(fileno(file), total_len) != 0) - { - throw std::logic_error(fmt::format( - "Failed to truncate ledger: {}", ccf::nonstd::strerror(errno))); - } - } - fseeko(file, total_len, SEEK_SET); LOG_TRACE_FMT("Truncated ledger file {} at seqno {}", file_name, idx); return false; @@ -595,21 +657,10 @@ namespace asynchost { return; } - // It may happen (e.g. during recovery) that the incomplete ledger gets - // truncated on the primary, so we have to make sure that whenever we - // complete the file it doesn't contain anything past the last_idx, which - // can happen on the follower unless explicitly truncated before - // completion. This is only necessary when the file was recovered from an - // existing file on disk (from_existing_file is true). For fresh files, - // total_len always matches the physical file size, so avoid a potentially - // expensive truncate. - if (from_existing_file) - { - truncate(get_last_idx(), /* remove_file_if_empty = */ false); - } - fseeko(file, total_len, SEEK_SET); size_t table_offset = ftello(file); + const auto completed_file_size = + table_offset + positions.size() * sizeof(positions.at(0)); { TimeBoundLogger log_if_slow(fmt::format( @@ -653,6 +704,8 @@ namespace asynchost } } + truncate_physical_file(completed_file_size); + LOG_TRACE_FMT("Completed ledger file {}", file_name); completed = true; diff --git a/src/host/test/ledger.cpp b/src/host/test/ledger.cpp index 83f3abe923b..c725eec52d4 100644 --- a/src/host/test/ledger.cpp +++ b/src/host/test/ledger.cpp @@ -134,6 +134,20 @@ size_t number_of_recovery_files_in_ledger_dir() return recovery_file_count; } +fs::path get_only_ledger_file_path() +{ + fs::path ledger_file; + size_t file_count = 0; + for (auto const& f : fs::directory_iterator(ledger_dir)) + { + ledger_file = f.path(); + file_count++; + } + + REQUIRE(file_count == 1); + return ledger_file; +} + void verify_framed_entries_range( const asynchost::LedgerReadResult& read_result, size_t from, size_t to) { @@ -899,6 +913,37 @@ TEST_CASE("Truncation") } } +TEST_CASE("Truncation defers physical file shrink") +{ + auto dir = AutoDeleteFolder(ledger_dir); + + Ledger ledger(ledger_dir, wf); + TestEntrySubmitter entry_submitter(ledger, 1024); + + for (size_t i = 0; i < 5; ++i) + { + entry_submitter.write(true); + } + + const auto ledger_file_path = get_only_ledger_file_path(); + const auto original_file_size = fs::file_size(ledger_file_path); + + entry_submitter.truncate(2); + REQUIRE(fs::file_size(ledger_file_path) == original_file_size); + + { + Ledger restored_ledger(ledger_dir, wf); + read_entries_range_from_ledger(restored_ledger, 1, 2); + REQUIRE(restored_ledger.get_last_idx() == 2); + } + + TestEntrySubmitter truncation_submitter(ledger, 1024, 2); + truncation_submitter.write(true, ccf::kv::FORCE_LEDGER_CHUNK_AFTER); + ledger.commit(3); + + REQUIRE(fs::file_size(get_only_ledger_file_path()) < original_file_size); +} + TEST_CASE("Commit") { auto dir = AutoDeleteFolder(ledger_dir); From 19005e431d5b73da26d63f3a895957f841b4339d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Jun 2026 21:14:54 +0000 Subject: [PATCH 3/6] Address ledger truncation validation feedback Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com> --- src/host/ledger.h | 8 ++++++-- src/host/test/ledger.cpp | 9 ++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/host/ledger.h b/src/host/ledger.h index f5db56991fd..b606655124f 100644 --- a/src/host/ledger.h +++ b/src/host/ledger.h @@ -89,6 +89,9 @@ namespace asynchost FILE* file = nullptr; ccf::pal::Mutex file_lock; + static constexpr uint64_t truncation_marker_size = + (1ULL << ccf::kv::SerialisedEntryHeader::BITS_FOR_SIZE) - 1; + size_t start_idx = 1; size_t total_len = 0; // Points to end of last written entry std::vector positions; @@ -145,6 +148,8 @@ namespace asynchost void write_truncation_marker(size_t physical_size) { + // If there is no complete entry header beyond the logical end, recovery + // will already stop at total_len. if (physical_size < total_len + ccf::kv::serialised_entry_header_size) { return; @@ -159,8 +164,7 @@ namespace asynchost } ccf::kv::SerialisedEntryHeader marker; - marker.set_size( - (1ULL << ccf::kv::SerialisedEntryHeader::BITS_FOR_SIZE) - 1); + marker.set_size(truncation_marker_size); TimeBoundLogger log_if_slow(fmt::format( "Writing ledger truncation marker - fwrite({})", file_name)); diff --git a/src/host/test/ledger.cpp b/src/host/test/ledger.cpp index c725eec52d4..aee5cbc0847 100644 --- a/src/host/test/ledger.cpp +++ b/src/host/test/ledger.cpp @@ -140,7 +140,10 @@ fs::path get_only_ledger_file_path() size_t file_count = 0; for (auto const& f : fs::directory_iterator(ledger_dir)) { - ledger_file = f.path(); + if (file_count == 0) + { + ledger_file = f.path(); + } file_count++; } @@ -937,8 +940,8 @@ TEST_CASE("Truncation defers physical file shrink") REQUIRE(restored_ledger.get_last_idx() == 2); } - TestEntrySubmitter truncation_submitter(ledger, 1024, 2); - truncation_submitter.write(true, ccf::kv::FORCE_LEDGER_CHUNK_AFTER); + TestEntrySubmitter post_truncation_submitter(ledger, 1024, 2); + post_truncation_submitter.write(true, ccf::kv::FORCE_LEDGER_CHUNK_AFTER); ledger.commit(3); REQUIRE(fs::file_size(get_only_ledger_file_path()) < original_file_size); From 03c4723c0790c763571ee47e5dc33b21be2b9f30 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Jun 2026 21:16:17 +0000 Subject: [PATCH 4/6] Refine ledger truncation helpers Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com> --- src/host/ledger.h | 11 ++++++++++- src/host/test/ledger.cpp | 6 +++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/host/ledger.h b/src/host/ledger.h index b606655124f..2910ad6b752 100644 --- a/src/host/ledger.h +++ b/src/host/ledger.h @@ -137,9 +137,18 @@ namespace asynchost return; } + const auto fd = fileno(file); + if (fd == -1) + { + throw std::logic_error(fmt::format( + "Failed to get file descriptor for ledger file {}: {}", + file_name, + ccf::nonstd::strerror(errno))); + } + TimeBoundLogger log_if_slow( fmt::format("Truncating ledger file - ftruncate({})", file_name)); - if (ftruncate(fileno(file), size) != 0) + if (ftruncate(fd, size) != 0) { throw std::logic_error(fmt::format( "Failed to truncate ledger: {}", ccf::nonstd::strerror(errno))); diff --git a/src/host/test/ledger.cpp b/src/host/test/ledger.cpp index aee5cbc0847..1fc564da1b5 100644 --- a/src/host/test/ledger.cpp +++ b/src/host/test/ledger.cpp @@ -134,7 +134,7 @@ size_t number_of_recovery_files_in_ledger_dir() return recovery_file_count; } -fs::path get_only_ledger_file_path() +fs::path get_single_ledger_file_path() { fs::path ledger_file; size_t file_count = 0; @@ -928,7 +928,7 @@ TEST_CASE("Truncation defers physical file shrink") entry_submitter.write(true); } - const auto ledger_file_path = get_only_ledger_file_path(); + const auto ledger_file_path = get_single_ledger_file_path(); const auto original_file_size = fs::file_size(ledger_file_path); entry_submitter.truncate(2); @@ -944,7 +944,7 @@ TEST_CASE("Truncation defers physical file shrink") post_truncation_submitter.write(true, ccf::kv::FORCE_LEDGER_CHUNK_AFTER); ledger.commit(3); - REQUIRE(fs::file_size(get_only_ledger_file_path()) < original_file_size); + REQUIRE(fs::file_size(get_single_ledger_file_path()) < original_file_size); } TEST_CASE("Commit") From f8710232bf4d101f576889b2d4d14d429fcafea5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Jun 2026 21:17:40 +0000 Subject: [PATCH 5/6] Clarify ledger truncation marker handling Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com> --- src/host/ledger.h | 29 ++++++++++++++++++++++++++--- src/host/test/ledger.cpp | 7 ++++--- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/host/ledger.h b/src/host/ledger.h index 2910ad6b752..4b3bb4940b9 100644 --- a/src/host/ledger.h +++ b/src/host/ledger.h @@ -173,6 +173,9 @@ namespace asynchost } ccf::kv::SerialisedEntryHeader marker; + // Use the largest encodable entry size so recovery sees a complete + // header whose payload cannot fit in the remaining file tail, and stops + // before recovering stale entries. marker.set_size(truncation_marker_size); TimeBoundLogger log_if_slow(fmt::format( @@ -659,7 +662,13 @@ namespace asynchost } } - fseeko(file, total_len, SEEK_SET); + if (fseeko(file, total_len, SEEK_SET) != 0) + { + throw std::logic_error(fmt::format( + "Failed to seek to logical end of ledger file {}: {}", + file_name, + ccf::nonstd::strerror(errno))); + } LOG_TRACE_FMT("Truncated ledger file {} at seqno {}", file_name, idx); return false; } @@ -670,8 +679,22 @@ namespace asynchost { return; } - fseeko(file, total_len, SEEK_SET); - size_t table_offset = ftello(file); + if (fseeko(file, total_len, SEEK_SET) != 0) + { + throw std::logic_error(fmt::format( + "Failed to seek to positions table offset in ledger file {}: {}", + file_name, + ccf::nonstd::strerror(errno))); + } + const auto table_offset_ = ftello(file); + if (table_offset_ < 0) + { + throw std::logic_error(fmt::format( + "Failed to read positions table offset in ledger file {}: {}", + file_name, + ccf::nonstd::strerror(errno))); + } + const auto table_offset = static_cast(table_offset_); const auto completed_file_size = table_offset + positions.size() * sizeof(positions.at(0)); diff --git a/src/host/test/ledger.cpp b/src/host/test/ledger.cpp index 1fc564da1b5..ee00ebd595c 100644 --- a/src/host/test/ledger.cpp +++ b/src/host/test/ledger.cpp @@ -134,7 +134,7 @@ size_t number_of_recovery_files_in_ledger_dir() return recovery_file_count; } -fs::path get_single_ledger_file_path() +fs::path require_single_ledger_file_path() { fs::path ledger_file; size_t file_count = 0; @@ -928,7 +928,7 @@ TEST_CASE("Truncation defers physical file shrink") entry_submitter.write(true); } - const auto ledger_file_path = get_single_ledger_file_path(); + const auto ledger_file_path = require_single_ledger_file_path(); const auto original_file_size = fs::file_size(ledger_file_path); entry_submitter.truncate(2); @@ -944,7 +944,8 @@ TEST_CASE("Truncation defers physical file shrink") post_truncation_submitter.write(true, ccf::kv::FORCE_LEDGER_CHUNK_AFTER); ledger.commit(3); - REQUIRE(fs::file_size(get_single_ledger_file_path()) < original_file_size); + REQUIRE( + fs::file_size(require_single_ledger_file_path()) < original_file_size); } TEST_CASE("Commit") From 3cf616decd9b51778b6f35239ffc1262a553d0fd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Jun 2026 21:19:03 +0000 Subject: [PATCH 6/6] Improve ledger truncation diagnostics Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com> --- src/host/ledger.h | 13 ++++++++----- src/host/test/ledger.cpp | 4 ++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/host/ledger.h b/src/host/ledger.h index 4b3bb4940b9..8a5398592a8 100644 --- a/src/host/ledger.h +++ b/src/host/ledger.h @@ -167,7 +167,9 @@ namespace asynchost if (fseeko(file, total_len, SEEK_SET) != 0) { throw std::logic_error(fmt::format( - "Failed to seek to truncation marker in ledger file {}: {}", + "Failed to seek to truncation marker at logical end {} in ledger " + "file {}: {}", + total_len, file_name, ccf::nonstd::strerror(errno))); } @@ -183,7 +185,8 @@ namespace asynchost if (fwrite(&marker, sizeof(marker), 1, file) != 1) { throw std::logic_error(fmt::format( - "Failed to write truncation marker to ledger file {}: {}", + "Failed to write {}-byte truncation marker to ledger file {}: {}", + sizeof(marker), file_name, ccf::nonstd::strerror(errno))); } @@ -686,15 +689,15 @@ namespace asynchost file_name, ccf::nonstd::strerror(errno))); } - const auto table_offset_ = ftello(file); - if (table_offset_ < 0) + const auto raw_table_offset = ftello(file); + if (raw_table_offset < 0) { throw std::logic_error(fmt::format( "Failed to read positions table offset in ledger file {}: {}", file_name, ccf::nonstd::strerror(errno))); } - const auto table_offset = static_cast(table_offset_); + const auto table_offset = static_cast(raw_table_offset); const auto completed_file_size = table_offset + positions.size() * sizeof(positions.at(0)); diff --git a/src/host/test/ledger.cpp b/src/host/test/ledger.cpp index ee00ebd595c..7c0a497ab3b 100644 --- a/src/host/test/ledger.cpp +++ b/src/host/test/ledger.cpp @@ -145,6 +145,10 @@ fs::path require_single_ledger_file_path() ledger_file = f.path(); } file_count++; + if (file_count > 1) + { + break; + } } REQUIRE(file_count == 1);