docs(testing): a T3 matrix whose timings mean something, and a corpus server that does not truncate - #1193
Draft
eterceros wants to merge 5 commits into
Draft
docs(testing): a T3 matrix whose timings mean something, and a corpus server that does not truncate#1193eterceros wants to merge 5 commits into
eterceros wants to merge 5 commits into
Conversation
… server that does not truncate T3 of the manual matrix ran >70 h in the #937 pass without reaching its pass criteria, and the numbers it did produce could not be trusted: §3 built a debug binary while every step ran `./target/release/hfs`, §7.1 prescribed `python3 -m http.server` for a 35 GB corpus, and §4 sized Elasticsearch for T2. `MANUAL_TESTING_MATRIX_V2.md` is a candidate revision — v1 is untouched and stays the document of record until a full pass has been run against this one. It opens with a "what changed from v1, and why" table, one row per change with the measurement behind it, and changes only the T3 path: - §3: one profile, `--release`, in the build command and in both places that launch the binary, plus a timestamp check on the binary (the failure this replaces was a tester silently running a stale release build). 3.b documents the R4-only `-p helios-hfs` build with its cold/incremental times, one-cargo-at-a-time, and why `sccache` 0.17.0 stays out. The `--all-features` enumeration now includes `skip-r6-download`, which is why neither build touches the R6 fixtures. - §4/§4.1: 8 GB heap, a named volume, a per-run `HFS_ELASTICSEARCH_INDEX_PREFIX`, a shard pre-flight, `number_of_replicas=0` on the live indices (there is no env var for it), and a cleanup that deletes by name because ES 8 rejects wildcard deletes. The 1,000-shard wall is documented as what it looks like — an HFS that never becomes ready — so it is not filed again. - §5: separate load (T3) and search (T4) profiles with the restart between them; `HFS_ELASTICSEARCH_WRITE_REFRESH=wait_for` took the rebuild from 255 to 114 resources/s on a 1 % cut. Plus what the detail page's 5 s poll costs at 19M rows and why the dashboard is safe to leave open since #1081. - §2/§7.1: an HTTP/1.1 keep-alive static server instead of `python3 -m http.server`, with a byte-for-byte check of the served corpus as a step. The §12.1 rest-hook receiver keeps its stdlib server; small JSON POSTs are not the defect. - §7.3: judge progress by the resource counter, not the byte percentage capped at 99; expect `total_entries` to equal the corpus size; the log lines that are failures, quoted from `reindex.rs` and the submit worker. - §7.4 (new): wait for the deferred rebuild, confirm with `$reindex-status`, record its elapsed time separately. §7.5's counts are compared per type and a shortfall is a failure — the only check that catches the Provenance shortfall — and Elasticsearch counts come from `_count` with `must_not is_contained` rather than `_cat/indices`, whose `docs.count` includes Lucene's nested documents (#991). - §14: the ingest time and the searchable time as two numbers, plus the final database size. §15: the shard wall, "Processing 99 %", Completed vs searchable. The benchmark scripts had the same truncation defect. `FixtureProvider` in `measure_memory.py` and the fixture server in `measure_reindex_coordination.py` now pin `protocol_version = "HTTP/1.1"` with a connection timeout; the stdlib default, HTTP/1.0, closes the socket after every response and lost 20-130 KB off the tail of 4-8 files per run while the submission still reported `completed` — a benchmark over input HFS never fully received. `--provider-url` hands the corpus to an external static server instead, probed before measuring so a misrooted docroot fails as a config error rather than as a mid-job fetch failure, and `run.json` records under `config.corpus_provider` which server served the run. Verified: `python3 -m py_compile` and `--help`/`--dry-run` on both scripts; a functional keep-alive test against both fixture servers (repeated full-body reads of a 31-40 MB file on one HTTP/1.1 connection, 8 concurrent readers, and recovery after an abandoned connection — no truncation); `--provider-url` accept/reject paths; and every code reference quoted in the matrix checked against the file and line it names. No Rust code changed and v1 is byte-identical to its committed state. Closes #1126 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… banner that is a result The Playwright suite could not run: `helios-persistence` did not compile. #1156 and #1158 each claimed SQLite schema v30 (`index_pending` and the `resource_key` surrogate), so once both were on `main` the merge had `migrate_v29_to_v30` defined twice. That is `main`'s breakage, not this branch's — it is already present in the pure-merge commit this branch sits on — and `main` has since fixed it in #1164 (`824ca3c91`, resource_key becomes v30→v31, `SCHEMA_VERSION = 31`). This branch therefore merges `origin/main` rather than writing a second, divergent fix: the numbering is not free, because databases stamped v30 between the two merges already have `index_pending` and still need the `resource_key` step. With #1156 merged, the matrix's §5 advice was out of date, so `MANUAL_TESTING_MATRIX_V2.md` follows it: - §5 is now **one environment for the whole pass** instead of a load profile, a search profile and a restart between them. `HFS_ELASTICSEARCH_REINDEX_REFRESH` (`crates/rest/src/config.rs:1193`) sets the refresh policy of `$reindex` and of the deferred rebuild separately from ordinary writes, falling back to `HFS_ELASTICSEARCH_WRITE_REFRESH` when unset, so the pass keeps read-your-write for T2/T4 *and* a rebuild that does not wait: 806 s → 145 s (1,576 resources/s, 0 errors) on the 228,580-resource cut, the figures `crates/persistence/README.md:1531` records. Documented with its three accepted values, that a bad value is a startup error, that it reaches only the Elasticsearch rebuild writer, and that it does not change the import's ordinary writes. The T3 load settings move into the same block. - §7.3, §7.4 and §15: the rebuild banner is now a **result**, not only a progress bar — since #1156 it stays up when the last rebuild left resources unindexed. §7.4 carries the full outcome table read off `reindex_activity_of` (`crates/rest/src/dashboard.rs:623`): hidden for a clean completion, for a *cancelled* rebuild and for one that never ran; visible, with different wording, for a failure and for a completion with per-resource errors. Its absence still has to be confirmed with `$reindex-status`, and the banner is evicted with the job's status after 24 h. - §7.4 also records that #1156 is the fix for the Provenance shortfall the campaign hit, so a shortfall now means a regression against a measured baseline. - §5 gains `HFS_REINDEX_BATCH_BYTES` as the knob for a rebuild that fails on oversized resources, and the non-composite note now names `s3` too. Verified: `cargo build -p helios-hfs --features ui` green, which is the failure that was reported. `cargo test -p helios-persistence --lib backends::sqlite::schema`: 22 passed, including `test_migration_ladder_replays_on_a_current_database`, the test that catches a missing ladder arm. `cargo test -p helios-ui --test rebuild_banner_http`: 4 passed, including `home_and_resources_keep_saying_when_the_last_rebuild_left_resources_unindexed`, which asserts the banner behavior this commit documents. `cargo test -p helios-rest --lib dashboard::`: 40 passed, including the assertion that a cancelled rebuild shows no banner. Playwright e2e (`CI=1 HFS_E2E_PORT=18300`): **482 passed, 7 skipped, 1 flaky**, exit 0. The flaky one is real but pre-existing and unrelated to this branch, which changes no Rust, template or CSS: `design-system.spec.ts` "every class used on every page matches a rule in app.css" caught `/ui/resources: .count--approximate`. `type_rail.html:12` and `rail_recent.html:39` render that modifier on approximate rail counts (#1081) and `crates/ui/assets/app.css` has no rule for it, so the marker styles nothing. It is intermittent because the class only appears while counts are unreconciled, which is why the retry passed. Worth its own issue. Every `path:line` citation in the matrix was re-resolved against the merged tree: 29 checked, 0 broken. No Rust source was written by this commit. Closes #1126 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…it called a server defect #1159 (#1127) landed on this branch after the matrix was written and changed three things it asserts, plus the line numbers of nine of its citations. What was wrong: - §7.1 said a truncated corpus file leaves "the manifest still ends `completed`, with no WARN in the log". That was the measured behaviour and is now fixed: HFS re-requests the remainder with a `Range` request (`crates/rest/src/bulk_submit_fetcher.rs:731`) and, failing that, a manifest with any input file that could not be read to its end is `failed`, never `completed` (`crates/persistence/src/core/bulk_submit_worker.rs:1864`). The ban on `python3 -m http.server` stands, but for a different reason — a truncating server now costs hours of ingest and a failed run instead of a silently short database, so the byte check still earns its place by failing in minutes. §15 and the changelog row say the same. - §7.3 said a re-walk makes `total_entries` report a multiple of the corpus size. Schema v32 keeps a per-file high-water mark in `bulk_manifest_file_progress` (`sqlite/bulk_submit.rs:394`), so the counters no longer multiply; equality with the corpus size is now a real check and a multiple is a defect to report. The re-walk itself is still possible and still costs hours, so the lease row now says to record it from the clock. The pre-v32 exception is named (`sqlite/schema.rs:1508`). - §7.4's parenthetical guess that "if the server change that indexes during ingest lands, this phase disappears" is now a real, opt-in flag: `HFS_BULK_SUBMIT_INDEX_DURING_INGEST` (`crates/rest/src/config.rs:891`, default `false`). §5 and §7.4 document it, its four tunables, that the pass exercises the default, and that whichever mode was run must be recorded — the searchable time in §14 means different things in each. Four new failure log lines from the fetcher and the worker join §7.3's table, and the status poll's SQL is re-quoted in its current shape (five aggregates, now scoped by `manifest_id`). The three docstrings in the benchmark scripts that repeated the "still ended completed" claim are put in the past tense with the current behaviour next to them. Verified: all 37 `path:line` citations in the matrix re-resolve against this tree (the nine that #1159 shifted are renumbered); `python3 -m py_compile` clean on both scripts. Documentation only — no Rust changed. Closes #1126
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1126
Fixes T3 of the manual release matrix, whose instructions are currently self-contradictory: §3 builds a debug binary while every step launches ./target/release/hfs, §4 sizes Elasticsearch for T2 (1g heap, no volume) and hits the 1000-shard wall that looks like an HFS startup bug, §5 applies one env profile where T3 (load) and T4 (search) need opposite settings, and §7.1 prescribes python3 -m http.server, which truncates multi-gigabyte NDJSON so the import can report Completed over a database missing thousands of resources. Scope is MANUAL_TESTING_MATRIX.md plus the isolated FixtureProvider HTTP server in crates/hfs/tests/bulk_submit/measure_memory.py (no production Rust, no migrations). While verifying the issue's line references against the tree, three claims were found stale and are corrected in the rewrite: the rebuild banner persists as failed rather than disappearing, the config.rs line numbers moved (1122 and 1154), and two reindex log messages were quoted without their trailing clauses.
PR auto-generated by claude-agent; fix implemented with Claude Code and validated with the Playwright e2e suite.