diff --git a/CHANGELOG.md b/CHANGELOG.md index 30cbca2c3aa..dc48fb2e826 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - HTTP messages (requests or replies) whose `Content-Length` header advertises a body larger than the configured maximum body size are now rejected as soon as the headers have been parsed, rather than after enough body chunks have been received to exceed the limit (#8045). - The thread-identifier helpers used by `ccf/ds/logger.h` (`ccf::threading::get_current_thread_id`, `set_current_thread_id`, and `reset_thread_id_generator`) have moved out of `libccf` into a new standalone `ccf_threading` static library, which `find_package(ccf)` exports automatically. This removes a long-standing implicit circular dependency (#7977). - **Build-graph change for consumers that link CCF component libraries directly.** `ccfcrypto` now links the new `ccf_threading` library, and `ccf_tasks` and `ccf_kv` link `ccf_threading` directly instead of `ccfcrypto`. Downstream targets that linked `ccf_tasks` or `ccf_kv` directly and relied on them transitively supplying CCF cryptography must now link `ccfcrypto` explicitly. Applications built with `add_ccf_app` (which link `ccf` and `ccf_launcher`) are unaffected (#7977). +- Nodes that start from a snapshot (a backup joining a network, or a node recovering) no longer suffer degraded ledger write latency on Azure Files/SMB (CIFS) mounts whose client kernel lacks the compound-operation lease-key fix (e.g. Linux 6.1). Such nodes write entries received after the snapshot into temporary `.recovery` ledger chunks and rename them to their final names once the service is open; the active (uncommitted) chunks are now closed before that rename and reopened immediately afterwards, so the writable handle reacquires a fresh write-caching lease instead of writing through to the server on every subsequent entry (#8071). ## [7.0.9] diff --git a/src/host/ledger.h b/src/host/ledger.h index 432f6d1a534..ab5a1bebc81 100644 --- a/src/host/ledger.h +++ b/src/host/ledger.h @@ -684,8 +684,62 @@ namespace asynchost void open() { auto new_file_name = remove_recovery_suffix(file_name.c_str()); - rename(new_file_name); + auto file_path = dir / file_name; + auto new_file_path = dir / new_file_name; + + if (!committed) + { + // Uncommitted files may be truncated and written again after recovery. + // Close before the rename and reopen afterwards so affected CIFS + // clients acquire a fresh write lease. + { + TimeBoundLogger log_if_slow(fmt::format( + "Closing recovery ledger file - fclose({})", file_name)); + errno = 0; + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) + const auto close_rc = fclose(file); + const auto close_errno = errno; + file = nullptr; + if (close_rc != 0) + { + throw std::logic_error(fmt::format( + "Failed to close recovery ledger file {}: {}", + file_path, + ccf::nonstd::strerror(close_errno != 0 ? close_errno : EIO))); + } + } + } + + { + TimeBoundLogger log_if_slow(fmt::format( + "Renaming ledger file {} to {} - rename()", + file_name, + new_file_name)); + files::rename(file_path, new_file_path); + } + file_name = new_file_name; recovery = false; + + if (!committed) + { + int open_errno = 0; + { + TimeBoundLogger log_if_slow(fmt::format( + "Reopening recovery ledger file - fopen({})", new_file_path)); + errno = 0; + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) + file = fopen(new_file_path.c_str(), "r+b"); + open_errno = errno; + } + if (file == nullptr) + { + throw std::logic_error(fmt::format( + "Failed to reopen recovery ledger file {}: {}", + new_file_path, + ccf::nonstd::strerror(open_errno != 0 ? open_errno : EIO))); + } + } + LOG_DEBUG_FMT("Open recovery ledger file {}", new_file_name); } diff --git a/src/host/test/ledger.cpp b/src/host/test/ledger.cpp index 312980b93ab..87ce20184e2 100644 --- a/src/host/test/ledger.cpp +++ b/src/host/test/ledger.cpp @@ -1840,6 +1840,68 @@ TEST_CASE("Recovery") read_entry_from_ledger(ledger, recovery_idx + 1); } + SUBCASE("Reopen active file when completing recovery") + { + Ledger ledger(ledger_dir, wf); + TestEntrySubmitter entry_submitter(ledger, chunk_threshold); + + initialise_ledger(entry_submitter, entries_per_chunk, 1); + ledger.commit(entry_submitter.get_last_idx()); + + ledger.set_recovery_start_idx(entry_submitter.get_last_idx()); + entry_submitter.write(true); + REQUIRE(number_of_recovery_files_in_ledger_dir() == 1); + + const auto file_count = number_of_files_in_ledger_dir(); + const auto fd_count = number_open_fd(); + ledger.complete_recovery(); + + REQUIRE(number_of_recovery_files_in_ledger_dir() == 0); + REQUIRE(number_of_files_in_ledger_dir() == file_count); + REQUIRE(number_open_fd() == fd_count); + + entry_submitter.write(true); + const auto post_recovery_idx = entry_submitter.get_last_idx(); + REQUIRE(number_of_files_in_ledger_dir() == file_count); + read_entry_from_ledger(ledger, post_recovery_idx); + + entry_submitter.write(true, ccf::kv::EntryFlags::FORCE_LEDGER_CHUNK_AFTER); + ledger.commit(entry_submitter.get_last_idx()); + read_entries_range_from_ledger(ledger, 1, entry_submitter.get_last_idx()); + } + + SUBCASE("Reopen uncommitted completed file when completing recovery") + { + Ledger ledger(ledger_dir, wf); + TestEntrySubmitter entry_submitter(ledger, chunk_threshold); + + initialise_ledger(entry_submitter, entries_per_chunk, 1); + ledger.commit(entry_submitter.get_last_idx()); + + ledger.set_recovery_start_idx(entry_submitter.get_last_idx()); + entry_submitter.write(true); + const auto first_recovery_idx = entry_submitter.get_last_idx(); + entry_submitter.write(true, ccf::kv::EntryFlags::FORCE_LEDGER_CHUNK_AFTER); + REQUIRE(number_of_recovery_files_in_ledger_dir() == 1); + + const auto file_count = number_of_files_in_ledger_dir(); + const auto fd_count = number_open_fd(); + ledger.complete_recovery(); + + REQUIRE(number_of_recovery_files_in_ledger_dir() == 0); + REQUIRE(number_of_files_in_ledger_dir() == file_count); + REQUIRE(number_open_fd() == fd_count); + + entry_submitter.truncate(first_recovery_idx); + entry_submitter.write(true); + REQUIRE(number_of_files_in_ledger_dir() == file_count); + read_entry_from_ledger(ledger, entry_submitter.get_last_idx()); + + entry_submitter.write(true, ccf::kv::EntryFlags::FORCE_LEDGER_CHUNK_AFTER); + ledger.commit(entry_submitter.get_last_idx()); + read_entries_range_from_ledger(ledger, 1, entry_submitter.get_last_idx()); + } + SUBCASE("Enable and complete recovery") { Ledger ledger(ledger_dir, wf);