diff --git a/projects/clr/hipamd/src/hrr/hip_capture_writer.cpp b/projects/clr/hipamd/src/hrr/hip_capture_writer.cpp index c6756ea1fb28..7598e202ef0f 100644 --- a/projects/clr/hipamd/src/hrr/hip_capture_writer.cpp +++ b/projects/clr/hipamd/src/hrr/hip_capture_writer.cpp @@ -595,9 +595,6 @@ static ScanResult scan_events_for_resume(FILE* f, std::int64_t file_size) { break; } - r.max_seq = (h.sequence_id > r.max_seq) ? h.sequence_id : r.max_seq; - r.count++; - long body = static_cast(h.payload_length) - static_cast(hdr_size); // fseek past EOF "succeeds" on most platforms (it only fails to read on the // next fread), so a record claiming e.g. 65535 bytes of body in a 100-byte @@ -611,6 +608,10 @@ static ScanResult scan_events_for_resume(FILE* f, std::int64_t file_size) { r.append_at = pos; break; } + // Counted only once it fits: the torn record is cut off, so counting it + // would make the resumed trailer claim one record too many. + r.max_seq = (h.sequence_id > r.max_seq) ? h.sequence_id : r.max_seq; + r.count++; r.append_at = static_cast(ftell(f)); } diff --git a/projects/hrr/DESIGN.md b/projects/hrr/DESIGN.md index 871a377c42e7..82f39831ed2c 100644 --- a/projects/hrr/DESIGN.md +++ b/projects/hrr/DESIGN.md @@ -86,7 +86,9 @@ fault aborts the host, or a host SIGSEGV): - **Clean-shutdown trailer.** A normal `writer::flush` appends an `hrr_eof_record` (event_type `HRR_EOF_MARKER`, payload carries the final event count + `HRR_EOF_MAGIC`) and writes `manifest.json` with `complete:true`. The - trailer's **absence** is how the reader detects a crash-truncated archive. + reader takes an archive as complete only when the trailer is there and its + count matches the records before it; a missing trailer means the capture was + interrupted. Blobs and code objects were already crash-safe (written to a temp file then atomically renamed), so a crash never leaves a partial final blob. @@ -94,10 +96,18 @@ atomically renamed), so a crash never leaves a partial final blob. On the read side, `load_archive` is append-only-aware: a torn record can only be the last one, so a partial header/payload at the tail is a recovery point — all complete records are kept and `Archive::truncated` is set, rather than failing -the whole load. `Archive::complete` reflects whether the trailer was found. -`hrr-playback --info` reports both; `hrr-playback --repair` rewrites a truncated -archive (trimmed to the last complete record, trailer + manifest added) into a -clean one. +the whole load. The trailer ends the event stream. Records after it, which a +runtime older than the writer fix could leave from calls still in flight at +shutdown, are counted in `Archive::skipped_after_trailer`, reported once and +not replayed. `Archive::complete` is set when the trailer was found and its +`total_events` equals the number of records before it; a mismatch is reported +and leaves the archive incomplete without setting `truncated`. The trailer and +its count are kept in `Archive::trailer` and `Archive::trailer_events`, and +`hrr-playback --info` says which of these cases an archive is. `hrr-playback --repair` rewrites any +incomplete archive, whether its tail was torn, its trailer is missing or its +trailer miscounts, into a clean one: the complete records before the tail or +the trailer are kept, and a trailer that counts them and a manifest are +written. ### Per-Process Archive Layout @@ -149,8 +159,10 @@ sub-archive in turn and then rebuilds the root `manifest.json` from the results. This is the normal case for a multi-process serving stack, where the framework force-kills its workers at shutdown — the ranks that did all the GPU work are precisely the ones left without a trailer and absent from the root index, while -the parent that exited cleanly needs no repair. Sub-archives that already carry -a clean trailer are skipped without being read. +the parent that exited cleanly needs no repair. A sub-archive is skipped when +`hrr::has_clean_trailer`, a walk of its record headers that does not read the +payloads, finds it complete in the sense `load_archive` uses. `--info` on a +root applies the same check before it shows a process as complete. ### Archive Format (v8) ``` @@ -436,6 +448,8 @@ Version history, so an archive written by an older runtime can be placed: total_events u64 real events written before the trailer eof_magic u32 HRR_EOF_MAGIC ("HEOF") Absent => capture was interrupted (crash); reader recovers the tail. + Count != records before it => incomplete as well; --repair + rewrites either case. ``` ### `hrr_args_*` Struct Layout Rules diff --git a/projects/hrr/README.md b/projects/hrr/README.md index b1e1d65bc395..0d9fb99d94d5 100644 --- a/projects/hrr/README.md +++ b/projects/hrr/README.md @@ -100,7 +100,7 @@ On Linux the archive is readable only by the user who captured it (directories 0 | Option | Purpose | |--------|---------| | `--info` | Print archive summary and exit (no GPU) | -| `--repair` | Rewrite a crash-truncated archive with a clean trailer; on an archive root, repairs every process capture and rebuilds the root index | +| `--repair` | Rewrite an incomplete archive (torn tail, missing or miscounting trailer) with a clean trailer; on an archive root, repairs every process capture and rebuilds the root index | | `--events` | With `--info`: print the full event log | | `--verbose` | Print each event as it is replayed | | `--skip-device-sync` | Skip `hipDeviceSynchronize` / `hipStreamSynchronize` events | diff --git a/projects/hrr/playback/hrr_playback.cpp b/projects/hrr/playback/hrr_playback.cpp index 02b590ac38e9..6edb919e1b43 100644 --- a/projects/hrr/playback/hrr_playback.cpp +++ b/projects/hrr/playback/hrr_playback.cpp @@ -287,10 +287,13 @@ static bool print_root_info(const std::string& archive_path) { printf(" %-12s %-12s %-10s %-12s %-10s %s\n", "---", "----------", "--------", "------", "-----", "----"); for (const auto& [path, info] : processes) { + // The manifest says the process finalized; the trailer must also count the + // records before it, as load_archive requires. + const bool complete = info.complete && hrr::has_clean_trailer(path.string()); printf(" %-12llu %-12llu %-10s %-12llu %-10llu %s\n", static_cast(info.pid), static_cast(info.parent_pid), - info.complete ? "yes" : "NO", + complete ? "yes" : "NO", static_cast(info.event_count), static_cast(info.blob_count), path.string().c_str()); @@ -302,13 +305,21 @@ static bool print_root_info(const std::string& archive_path) { static void print_info(const hrr::Archive& archive, bool show_events) { printf("HRR Archive: %s\n", archive.path.c_str()); printf("========================================\n"); - printf("Complete: %s\n", - archive.complete ? "yes (clean shutdown)" - : (archive.truncated - ? "NO (crash-truncated; trailing torn record discarded)" - : "NO (no shutdown trailer; capture likely crashed)")); + if (archive.trailer && !archive.complete) + printf("Complete: NO (shutdown trailer counts %llu events; %zu precede it)\n", + static_cast(archive.trailer_events), + archive.event_count); + else + printf("Complete: %s\n", + archive.complete ? "yes (clean shutdown)" + : (archive.truncated + ? "NO (crash-truncated; trailing torn record discarded)" + : "NO (no shutdown trailer; capture likely crashed)")); printf("Recovered: %zu events\n", archive.event_count); printf("Events: %zu\n", archive.event_count); + if (archive.skipped_after_trailer) + printf("Ignored: %zu records after the shutdown trailer (not replayed)\n", + archive.skipped_after_trailer); printf("Kernels: %zu\n", archive.kernel_count); printf("Blobs: %zu\n", archive.blob_count); printf("Code Objects: %zu\n", archive.code_object_count); @@ -909,13 +920,16 @@ static bool run_pass(PlaybackContext& ctx, // --------------------------------------------------------------------------- // --------------------------------------------------------------------------- -// --repair mode: rewrite a crash-truncated archive into a clean one. +// --repair mode: rewrite an incomplete archive into a clean one. // -// Loads the archive with the tolerant reader (recovering all complete records), -// then rewrites events.bin trimmed to the last complete record, appends the -// clean-shutdown trailer, and writes a complete manifest.json. After repair the -// archive looks exactly like one produced by a normal process exit. Replay -// works on truncated archives without repair too; this just "blesses" them. +// An archive is incomplete when its tail is torn, its trailer is missing, or +// its trailer does not count the records before it. Loads the archive with the +// tolerant reader (recovering all complete records), then rewrites events.bin +// with the records before the torn tail or the trailer, appends a +// clean-shutdown trailer that counts them, and writes a complete manifest.json. +// After repair the archive looks exactly like one produced by a normal process +// exit. Replay works on incomplete archives without repair too; this just +// "blesses" them. // // repair_archive() below handles one process; repair_root() fans it out across // every pid-/ directory when the tool is pointed at an archive root. @@ -1028,29 +1042,6 @@ static int repair_archive(const hrr::Archive& archive) { return 0; } -// Cheap completeness probe: a cleanly finalized events.bin ends with the -// trailer record, so checking the last 44 bytes answers the question without -// parsing the file. Used only to skip work when repairing a root that is -// already clean — a false negative just costs a full load that repair_archive -// then no-ops on, so this can never turn a good archive into a rewritten one. -static bool has_clean_trailer(const fs::path& archive_dir) { - const fs::path events_path = archive_dir / "events.bin"; - std::error_code ec; - const auto size = fs::file_size(events_path, ec); - if (ec || size < sizeof(hrr_file_header) + sizeof(hrr_eof_record)) return false; - - FILE* f = fopen(events_path.string().c_str(), "rb"); - if (!f) return false; - hrr_eof_record rec{}; - const bool read_ok = fseek(f, -static_cast(sizeof(rec)), SEEK_END) == 0 && - fread(&rec, sizeof(rec), 1, f) == 1; - fclose(f); - - return read_ok && rec.hdr.event_type == HRR_EOF_MARKER && - rec.hdr.payload_length == static_cast(sizeof(hrr_eof_record)) && - rec.eof_magic == HRR_EOF_MAGIC; -} - // Rebuild the root index from the per-process manifests. Mirrors the schema the // capture writer emits (hip_capture_writer.cpp, write_root_manifest) so a // repaired archive is indistinguishable from a cleanly finalized one. @@ -1125,7 +1116,7 @@ static bool repair_root(const std::string& archive_path, int& exit_code) { // ahead of the whole run by block buffering when output is piped. fflush(stdout); - if (has_clean_trailer(dir)) { + if (hrr::has_clean_trailer(dir.string())) { printf("[HRR] Archive already complete; nothing to repair\n"); already_clean++; continue; @@ -1192,7 +1183,8 @@ static void print_usage(const char* argv0) { "\n" "Options:\n" " --info Print archive summary and exit (no GPU required)\n" - " --repair Rewrite a crash-truncated archive as a clean one and exit.\n" + " --repair Rewrite an incomplete archive (torn tail, missing or\n" + " miscounting trailer) as a clean one and exit.\n" " Given an archive root, repairs every process capture\n" " under it and rebuilds the root index.\n" " --events With --info: also print the full event log\n" @@ -1423,7 +1415,7 @@ int main(int argc, char** argv) { return 0; } - // --repair: rewrite a crash-truncated archive as a clean one and exit + // --repair: rewrite an incomplete archive as a clean one and exit if (do_repair) { return repair_archive(archive); } diff --git a/projects/hrr/playback/hrr_reader.cpp b/projects/hrr/playback/hrr_reader.cpp index 71d68e9def63..6ae4de5f71c3 100644 --- a/projects/hrr/playback/hrr_reader.cpp +++ b/projects/hrr/playback/hrr_reader.cpp @@ -18,12 +18,6 @@ namespace fs = std::filesystem; namespace hrr { -// payload_length is file-supplied. Without a ceiling, a corrupt header can ask -// resize() for ~4 GiB and OOM the process. Captured kernel launches are far -// smaller (the writer buffers 256 KiB and spills larger records to a direct -// write); 64 MiB is well above any legitimate record. -static constexpr uint32_t kMaxRecordBytes = 64u * 1024u * 1024u; - // --------------------------------------------------------------------------- // Event move semantics // --------------------------------------------------------------------------- @@ -324,9 +318,12 @@ bool load_archive(const std::string& path, Archive& archive) { // Read events sequentially. read_raw_record handles the framing and the // torn-tail recovery; a torn record means the capture was interrupted, and // everything parsed before it is kept. The clean-shutdown trailer - // (hrr_eof_record) explicitly marks a whole archive. + // (hrr_eof_record) explicitly marks a whole archive, and it ends the event + // stream. bool truncated = false; bool complete = false; + bool trailer = false; + uint64_t trailer_count = 0; const uint16_t hdr_size = static_cast(sizeof(hrr_event_header)); while (true) { Event ev; @@ -349,8 +346,9 @@ bool load_archive(const std::string& path, Archive& archive) { total == static_cast(sizeof(hrr_eof_record))) { const auto* er = reinterpret_cast(ev.raw_payload.data()); if (er->eof_magic == HRR_EOF_MAGIC) { - complete = true; - continue; // do not append trailer as a replay event + trailer = true; + trailer_count = er->total_events; + break; // the trailer is not a replay event, and nothing after it is } } @@ -598,11 +596,35 @@ bool load_archive(const std::string& path, Archive& archive) { archive.events.push_back(std::move(ev)); } + // A runtime older than the writer fix for it could append a few records + // after the trailer, from calls still in flight at shutdown. They are not in + // the trailer's count, so they are counted and left out of the replay. + if (trailer) { + std::vector rest; + while (read_raw_record(f, rest) == RecordStatus::Ok) + ++archive.skipped_after_trailer; + if (archive.skipped_after_trailer) + fprintf(stderr, + "[HRR] Ignored %zu records after the clean-shutdown trailer\n", + archive.skipped_after_trailer); + } + fclose(f); - archive.complete = complete; - archive.truncated = truncated; - if (!complete) { + if (trailer) { + complete = trailer_count == archive.events.size(); + if (!complete) + fprintf(stderr, + "[HRR] Clean-shutdown trailer counts %llu events but %zu precede it; " + "the archive is not treated as complete\n", + static_cast(trailer_count), archive.events.size()); + } + + archive.complete = complete; + archive.truncated = truncated; + archive.trailer = trailer; + archive.trailer_events = trailer_count; + if (!trailer) { fprintf(stderr, "[HRR] Archive has no clean-shutdown trailer (capture likely crashed); " "recovered %zu events%s\n", @@ -655,6 +677,49 @@ bool load_archive(const std::string& path, Archive& archive) { return true; } +// fseek takes a long, 32 bits on Windows, and an msvcrt fseek fails once the +// file position passes 2 GiB even for a small relative offset. events.bin can +// be larger than that, so the walk seeks with the 64-bit call. +static int seek64(FILE* f, int64_t offset, int origin) { +#ifdef _WIN32 + return _fseeki64(f, offset, origin); +#else + return fseeko(f, static_cast(offset), origin); +#endif +} + +// The framing checks are read_raw_record's, so a record the loader takes as +// torn ends the walk here too, and like the loader the walk stops at the first +// trailer. A payload that runs past the end of the file needs no check of its +// own: the seek succeeds and the next header read fails. +bool has_clean_trailer(const std::string& archive_dir) { + FILE* f = fopen((fs::path(archive_dir) / "events.bin").string().c_str(), "rb"); + if (!f) return false; + const uint32_t hdr_size = static_cast(sizeof(hrr_event_header)); + hrr_file_header fh{}; + bool ok = fread(&fh, sizeof(fh), 1, f) == 1 && fh.magic == HRR_MAGIC && + fh.version == HRR_VERSION; + uint64_t records = 0; + bool clean = false; + hrr_eof_record rec{}; + while (ok && fread(&rec.hdr, hdr_size, 1, f) == 1) { + const uint32_t total = rec.hdr.payload_length; + if (total < hdr_size || total > kMaxRecordBytes) break; + if (rec.hdr.event_type == HRR_EOF_MARKER && total == sizeof(rec)) { + ok = fread(reinterpret_cast(&rec) + hdr_size, sizeof(rec) - hdr_size, 1, f) == 1; + if (ok && rec.eof_magic == HRR_EOF_MAGIC) { + clean = rec.total_events == records; + break; + } + } else { + ok = seek64(f, static_cast(total - hdr_size), SEEK_CUR) == 0; + } + ++records; + } + fclose(f); + return clean; +} + // --------------------------------------------------------------------------- // Blob / code object readers // --------------------------------------------------------------------------- diff --git a/projects/hrr/playback/hrr_reader.h b/projects/hrr/playback/hrr_reader.h index 3e2b19c52e94..6c828faae2ed 100644 --- a/projects/hrr/playback/hrr_reader.h +++ b/projects/hrr/playback/hrr_reader.h @@ -166,14 +166,22 @@ struct Archive { std::vector events; // Crash-resilience status, set by load_archive: - // complete — the clean-shutdown trailer (hrr_eof_record) was found, so the - // capturing process exited normally and the archive is whole. + // complete — the clean-shutdown trailer (hrr_eof_record) was found and + // counts the records before it, so the capturing process + // exited normally and the archive is whole. // truncated — a torn trailing record was detected and discarded; all // complete records before it were recovered. Replay still works. // A capture interrupted by a crash typically has complete=false; it may also // have truncated=true if the final record was only partially written. bool complete = false; bool truncated = false; + // The trailer was found and how many events it counts. trailer && !complete + // means the count disagrees with the records before it. + bool trailer = false; + uint64_t trailer_events = 0; + // Whole records found after the trailer. They are not replayed, and the + // trailer's event count must match the records before it for complete=true. + size_t skipped_after_trailer = 0; // Content-addressed blobs: hash_hex -> file path std::unordered_map blobs; @@ -197,6 +205,11 @@ struct Archive { // Load an archive from disk. Returns false on error. bool load_archive(const std::string& path, Archive& archive); +// Whether load_archive would find the archive in `archive_dir` complete: a +// trailer whose total_events counts the records before it. Walks the record +// headers and seeks over the payloads, so it costs a fraction of a load. +bool has_clean_trailer(const std::string& archive_dir); + // --------------------------------------------------------------------------- // Record-stream primitives // @@ -206,6 +219,12 @@ bool load_archive(const std::string& path, Archive& archive); // shared framing, so both readers get the same torn-tail recovery. // --------------------------------------------------------------------------- +// payload_length is file-supplied. Without a ceiling, a corrupt header can ask +// resize() for ~4 GiB and OOM the process. Captured kernel launches are far +// smaller (the writer buffers 256 KiB and spills larger records to a direct +// write); 64 MiB is well above any legitimate record. A longer record is torn. +constexpr uint32_t kMaxRecordBytes = 64u * 1024u * 1024u; + enum class RecordStatus { Ok, // a complete record was read into the output buffer EndOfStream, // clean EOF exactly at a record boundary diff --git a/projects/hrr/tests/config/integration.yaml b/projects/hrr/tests/config/integration.yaml index 970785c630dc..4c367900ac74 100644 --- a/projects/hrr/tests/config/integration.yaml +++ b/projects/hrr/tests/config/integration.yaml @@ -63,6 +63,7 @@ cases: Unit_HRR_CaptureMarksUnwrittenFilesIncomplete: {} Unit_HRR_CaptureRefusesPlantedLinks: {} Unit_HRR_CaptureReplayRoundtrip: {} + Unit_HRR_CaptureResumeCountsOnlyWholeRecords: {} Unit_HRR_CaptureResumeTrustsOnlyItsOwnFiles: {} Unit_HRR_CaptureStartNotice: {} Unit_HRR_CaptureSurvivesUnusableOutputPath: {} diff --git a/projects/hrr/tests/config/unit.yaml b/projects/hrr/tests/config/unit.yaml index 0404378d3d2b..58621b8259ac 100644 --- a/projects/hrr/tests/config/unit.yaml +++ b/projects/hrr/tests/config/unit.yaml @@ -32,9 +32,16 @@ cases: Unit_HRR_Recovery_ImplausiblePayloadLength: {} Unit_HRR_Recovery_MultiProcessRootInfo: {} Unit_HRR_Recovery_NoTrailer: {} + Unit_HRR_Recovery_OversizeRecordBeforeTrailer: {} + Unit_HRR_Recovery_RecordAfterTrailer: {} + Unit_HRR_Recovery_RepairRootRewritesMiscountedTrailer: {} Unit_HRR_Recovery_RepairWritesProcessManifest: {} + Unit_HRR_Recovery_RootInfoChecksTrailerCount: {} Unit_HRR_Recovery_TornHeader: {} + Unit_HRR_Recovery_TornMiddleRecord: {} Unit_HRR_Recovery_TornPayload: {} + Unit_HRR_Recovery_TrailerCountMismatch: {} + Unit_HRR_Recovery_TrailerOvercounts: {} Unit_HRR_TranslatePtr_ExactMatch: {} Unit_HRR_TranslatePtr_OutOfRange_ReturnsNull: {} Unit_HRR_TranslatePtr_SubAlloc_ReturnsOffset: {} diff --git a/projects/hrr/tests/integration/hrr_capture_access_test.cc b/projects/hrr/tests/integration/hrr_capture_access_test.cc index 60b03322caba..b46dc1982653 100644 --- a/projects/hrr/tests/integration/hrr_capture_access_test.cc +++ b/projects/hrr/tests/integration/hrr_capture_access_test.cc @@ -53,6 +53,10 @@ * others makes them private again, and a hard-linked blob found there is * written again rather than trusted (POSIX). * + * Unit_HRR_CaptureResumeCountsOnlyWholeRecords: + * resuming an archive that ends in a torn record cuts it off and leaves + * it out of the event count, so the new trailer counts the records (POSIX). + * * Unit_HRR_CaptureActiveMarker: * pid-/active, the file producers read as "capture is on", exists * while the capture runs, names the process instance on Linux, and is @@ -902,6 +906,65 @@ HRR_TEST_CASE(Unit_HRR_CaptureResumeTrustsOnlyItsOwnFiles) { #endif } +// --------------------------------------------------------------------------- +/** + * Test Description + * ---------------- + * - Captures Unit_HRR_GpuWorkload_Direct, then copies its events.bin into + * the next run's pid- without the trailer and with a record header + * whose body was never written, as a crash mid-record leaves it. There is + * no writer_state.json, so the resume scans events.bin. + * - The resume cuts the torn record off and does not count it: the archive + * loads complete, with a trailer that counts its records. + */ +HRR_TEST_CASE(Unit_HRR_CaptureResumeCountsOnlyWholeRecords) { +#ifdef _WIN32 + HRR_SKIP("planting relies on /bin/sh and exec keeping the pid"); +#else + ScopedDir work{fs::temp_directory_path() / "hrr_access_resume_torn"}; + const fs::path first = work.path / "first"; + const fs::path base = work.path / "capture"; + const fs::path torn = work.path / "torn.bin"; + fs::create_directories(base); + hrr_capture_direct("Unit_HRR_GpuWorkload_Direct", first); + const fs::path first_archive = hrr_single_process_archive(first); + hrr::Archive earlier; + REQUIRE(hrr::load_archive(first_archive.string(), earlier)); + REQUIRE(earlier.complete); + REQUIRE(earlier.skipped_after_trailer == 0); + + fs::copy_file(first_archive / "events.bin", torn); + fs::resize_file(torn, fs::file_size(torn) - sizeof(hrr_eof_record)); + { + hrr_event_header h{}; + h.event_type = static_cast(HRR_API_HIPDEVICESYNCHRONIZE); + h.sequence_id = earlier.events.back().header().sequence_id + 1; + h.payload_length = 1024; + std::ofstream out(torn, std::ios::binary | std::ios::app); + out.write(reinterpret_cast(&h), sizeof(h)); + } + + const PlantedRun run = capture_after_planting( + base, work.path / "plant.sh", + "mkdir \"$HRR_TEST_BASE/pid-$$\"\n" + "cp -R '" + first_archive.string() + "/.' \"$HRR_TEST_BASE/pid-$$/\"\n" + "cp '" + torn.string() + "' \"$HRR_TEST_BASE/pid-$$/events.bin\"\n" + "rm -f \"$HRR_TEST_BASE/pid-$$/writer_state.json\"\n"); + INFO("Workload exit code: " << run.ret << "\n" << run.output); + REQUIRE(run.ret == 0); + CHECK(run.output.find(kCaptureDisabled) == std::string::npos); + const std::vector archives = hrr_process_archives(base); + REQUIRE(archives.size() == 1); + + hrr::Archive resumed; + REQUIRE(hrr::load_archive(archives.front().string(), resumed)); + CHECK(resumed.events.size() > earlier.events.size()); + CHECK(resumed.trailer); + CHECK(resumed.trailer_events == resumed.events.size()); + CHECK(resumed.complete); +#endif +} + // --------------------------------------------------------------------------- /** * Test Description diff --git a/projects/hrr/tests/unit/hrr_recovery_test.cc b/projects/hrr/tests/unit/hrr_recovery_test.cc index 4e42030fde5f..c993807c712d 100644 --- a/projects/hrr/tests/unit/hrr_recovery_test.cc +++ b/projects/hrr/tests/unit/hrr_recovery_test.cc @@ -110,8 +110,10 @@ struct TmpRootArchive { ~TmpRootArchive() { fs::remove_all(root); } + // trailer_count < 0 makes the trailer count the records written. fs::path add_process(uint64_t pid, uint64_t parent_pid, int records, - bool trailer = true, bool manifest = true) { + bool trailer = true, bool manifest = true, + int trailer_count = -1) { fs::path proc = root / ("pid-" + std::to_string(pid)); fs::create_directories(proc / "blobs"); fs::create_directories(proc / "code_objects"); @@ -124,8 +126,9 @@ struct TmpRootArchive { events.write(reinterpret_cast(&h), sizeof(h)); } if (trailer) { + const int counted = trailer_count < 0 ? records : trailer_count; hrr_eof_record eof = hrr_make_eof_record(static_cast(records), - static_cast(records)); + static_cast(counted)); events.write(reinterpret_cast(&eof), sizeof(eof)); } events.close(); @@ -157,6 +160,15 @@ static std::pair run_hrr_playback(const fs::path& archive, return {ret, proc.getOutput()}; } +// The row of `hrr-playback --info` for one process, up to the +// directory it ends in. +static std::string root_info_row(const std::string& out, const std::string& dir) { + const size_t at = out.find(dir); + if (at == std::string::npos) return std::string(); + const size_t bol = out.rfind('\n', at); + return out.substr(bol + 1, at - bol - 1); +} + } // namespace // --------------------------------------------------------------------------- @@ -180,6 +192,7 @@ HRR_TEST_CASE(Unit_HRR_Recovery_CompleteArchive) { CHECK(a.events.size() == 5); CHECK(a.complete); CHECK_FALSE(a.truncated); + CHECK(hrr::has_clean_trailer(arc.path())); } /** @@ -267,6 +280,149 @@ HRR_TEST_CASE(Unit_HRR_Recovery_NoTrailer) { CHECK_FALSE(a.truncated); } +/** + * Test Description + * ---------------- + * - N records, a clean trailer that counts them, then two more records, as a + * runtime that kept writing after the trailer at shutdown could leave. The + * reader stops at the trailer: the archive is complete with exactly the N + * records, and the two after it are counted and not replayed. + * - hrr::has_clean_trailer agrees that the archive is complete, and --info + * shows the two ignored records. + */ +HRR_TEST_CASE(Unit_HRR_Recovery_RecordAfterTrailer) { + TmpArchive arc("after_trailer"); + arc.write_records(5); + arc.write_trailer(5); + for (uint64_t seq : {100u, 101u}) { + hrr_event_header h = make_min_record(seq); + arc.write_bytes(&h, sizeof(h)); + } + arc.finish(); + + hrr::Archive a; + REQUIRE(hrr::load_archive(arc.path(), a)); + CHECK(a.events.size() == 5); + CHECK(a.complete); + CHECK_FALSE(a.truncated); + CHECK(a.skipped_after_trailer == 2); + for (const auto& ev : a.events) CHECK(ev.header().sequence_id < 100); + CHECK(hrr::has_clean_trailer(arc.path())); + + auto [ret, out] = run_hrr_playback(arc.root, "--info"); + INFO("hrr-playback --info stdout:\n" << out); + REQUIRE(ret == 0); + CHECK(out.find("Ignored: 2 records after the shutdown trailer") != std::string::npos); +} + +/** + * Test Description + * ---------------- + * - N records followed by a trailer that claims a different count. The + * trailer still ends the stream, but the archive is not treated as a clean + * complete capture, so --repair rewrites it rather than trusting it. + * - The archive keeps the trailer and its count, and --info reports the + * mismatch instead of a missing trailer. + */ +HRR_TEST_CASE(Unit_HRR_Recovery_TrailerCountMismatch) { + TmpArchive arc("count_mismatch"); + arc.write_records(5); + arc.write_trailer(4); + arc.finish(); + + hrr::Archive a; + REQUIRE(hrr::load_archive(arc.path(), a)); + CHECK(a.events.size() == 5); + CHECK_FALSE(a.complete); + CHECK_FALSE(a.truncated); + CHECK(a.skipped_after_trailer == 0); + CHECK(a.trailer); + CHECK(a.trailer_events == 4); + CHECK_FALSE(hrr::has_clean_trailer(arc.path())); + + auto [ret, out] = run_hrr_playback(arc.root, "--info"); + INFO("hrr-playback --info stdout:\n" << out); + REQUIRE(ret == 0); + CHECK(out.find("trailer counts 4 events; 5 precede it") != std::string::npos); + CHECK(out.find("no shutdown trailer") == std::string::npos); +} + +/** + * Test Description + * ---------------- + * - N records followed by a trailer that claims more than N. Like a trailer + * that claims fewer, the archive is not complete, and + * hrr::has_clean_trailer agrees. + */ +HRR_TEST_CASE(Unit_HRR_Recovery_TrailerOvercounts) { + TmpArchive arc("count_over"); + arc.write_records(5); + arc.write_trailer(6); + arc.finish(); + + hrr::Archive a; + REQUIRE(hrr::load_archive(arc.path(), a)); + CHECK(a.events.size() == 5); + CHECK_FALSE(a.complete); + CHECK_FALSE(hrr::has_clean_trailer(arc.path())); +} + +/** + * Test Description + * ---------------- + * - A record whose payload_length is shorter than a header, between whole + * records, and a trailer that counts every record in the file. The reader + * takes the short record as torn and stops there, so the archive is not + * complete, and hrr::has_clean_trailer agrees. + */ +HRR_TEST_CASE(Unit_HRR_Recovery_TornMiddleRecord) { + TmpArchive arc("torn_middle"); + arc.write_records(2); + hrr_event_header h = make_min_record(2); + h.payload_length = 16; + arc.write_bytes(&h, sizeof(h)); + for (uint64_t seq : {3u, 4u}) { + hrr_event_header r = make_min_record(seq); + arc.write_bytes(&r, sizeof(r)); + } + arc.write_trailer(5); + arc.finish(); + + hrr::Archive a; + REQUIRE(hrr::load_archive(arc.path(), a)); + CHECK(a.events.size() == 2); + CHECK(a.truncated); + CHECK_FALSE(a.complete); + CHECK_FALSE(hrr::has_clean_trailer(arc.path())); +} + +/** + * Test Description + * ---------------- + * - A record one byte longer than hrr::kMaxRecordBytes, whole on disk, then + * a trailer that counts it. The reader takes a record over the cap as + * torn, so the archive is not complete, and hrr::has_clean_trailer, which + * seeks over payloads instead of reading them, applies the same cap. + */ +HRR_TEST_CASE(Unit_HRR_Recovery_OversizeRecordBeforeTrailer) { + TmpArchive arc("oversize"); + arc.write_records(2); + hrr_event_header h = make_min_record(2); + h.payload_length = hrr::kMaxRecordBytes + 1; + arc.write_bytes(&h, sizeof(h)); + // Leave the body as a hole rather than writing 64 MiB. + arc.f.seekp(static_cast(h.payload_length - sizeof(h)), std::ios::cur); + arc.write_trailer(3); + arc.finish(); + + hrr::Archive a; + REQUIRE(hrr::load_archive(arc.path(), a)); + CHECK(a.events.size() == 2); + CHECK(a.truncated); + CHECK_FALSE(a.complete); + CHECK_FALSE(hrr::has_clean_trailer(arc.path())); +} + /** * Test Description * ---------------- @@ -312,6 +468,55 @@ HRR_TEST_CASE(Unit_HRR_Recovery_RepairWritesProcessManifest) { CHECK(manifest.find("\"capture_mode\"") == std::string::npos); } +/** + * Test Description + * ---------------- + * - A root with two sub-archives, one clean and one whose trailer sits at the + * end of events.bin but counts one record too few. + * - hrr-playback --repair on the root must not take the miscounting trailer + * as clean: it rewrites that sub-archive, which then loads complete with a + * trailer that counts its records. + */ +HRR_TEST_CASE(Unit_HRR_Recovery_RepairRootRewritesMiscountedTrailer) { + TmpRootArchive root("repair_root_miscount"); + root.add_process(/*pid=*/111, /*parent_pid=*/0, /*records=*/2); + fs::path bad = root.add_process(/*pid=*/222, /*parent_pid=*/111, + /*records=*/4, /*trailer=*/true, + /*manifest=*/true, /*trailer_count=*/3); + + auto [ret, out] = run_hrr_playback(root.root, "--repair"); + INFO("hrr-playback --repair stdout:\n" << out); + REQUIRE(ret == 0); + CHECK(out.find("1 repaired, 1 already complete, 0 failed") != std::string::npos); + + hrr::Archive repaired; + REQUIRE(hrr::load_archive(bad.string(), repaired)); + CHECK(repaired.complete); + CHECK(repaired.events.size() == 4); + CHECK(repaired.trailer_events == 4); +} + +/** + * Test Description + * ---------------- + * - A root with two sub-archives whose manifests both say complete, one of + * them with a trailer that counts one record too few. + * - hrr-playback --info shows that one as not complete, the other as + * complete. + */ +HRR_TEST_CASE(Unit_HRR_Recovery_RootInfoChecksTrailerCount) { + TmpRootArchive root("root_info_count"); + root.add_process(/*pid=*/111, /*parent_pid=*/0, /*records=*/2); + root.add_process(/*pid=*/222, /*parent_pid=*/111, /*records=*/4, + /*trailer=*/true, /*manifest=*/true, /*trailer_count=*/3); + + auto [ret, out] = run_hrr_playback(root.root, "--info"); + INFO("hrr-playback --info stdout:\n" << out); + REQUIRE(ret == 0); + CHECK(root_info_row(out, "pid-111").find(" yes ") != std::string::npos); + CHECK(root_info_row(out, "pid-222").find(" NO ") != std::string::npos); +} + /** * Test Description * ----------------