test: stabilize qualification closeout - #277
Merged
Merged
Conversation
smiggleworth
marked this pull request as ready for review
September 4, 2026 15:13
This was referenced Sep 4, 2026
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
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.
Summary
Close the remaining qualification gap after the cardinality-independence work. The Sqrzl-backed partitioned-compaction recovery and partial-upload rollback tests requested by #272 are already present on
main, workflow-wired, and green in exact-head Cloud Qualification.This change also repairs the qualification failures uncovered during that audit: both cloud-outage tests now inject failure at the real provider boundary instead of relying on Unix permissions that root can bypass; process-global upload injection is isolated from parallel tests; and the multi-output simulated-cloud durability qualification receives a bounded test-only timeout for hosted Windows filesystem contention.
Why are you making this contribution?
Issue #272 remained open after its requested coverage had landed, while current
mainhad a red Docker Qualification run and a red Windows Platform run. Exact-head validation of the first Docker fix then found a sibling chmod-based outage test and exposed the danger of process-global injection during parallel tests. Closing the issue without resolving those contradictions would leave the repository's actual qualification state inconsistent with the completed implementation.Linked issues
Closes #272
Acceptance audit
Criterion: A Sqrzl-backed test exercises multi-output compaction upload, publication, local-cache loss, and recovery.
Evidence:
should_recover_partitioned_compaction_from_sqrzl_s3_after_local_cache_losspassed in Cloud Qualification runs 33848658054 on priormainand 33888223535 on exact first PR headbc9138a32e0d0c4f322108a43e1de6a320f2c77a.Production entry point: the test opens the public Sqrzl S3 provider, drives
Engine::compact_all, removes the local cache, and reopens through normal manifest and SST recovery.Resolution: the requested real-emulator multi-output recovery proof exists and remains in the scheduled/manual workflow.
Criterion: A Sqrzl-backed failure-injection test proves that a partially mirrored replacement set is never authoritative.
Evidence:
should_rollback_partition_set_after_partial_sqrzl_compaction_uploadpassed in both cited Cloud Qualification runs withsqrzl-tests,failpoints.Production entry point: the test exercises partition upload, compaction intent, manifest authority, rollback, and reopen behavior against the Sqrzl S3 API.
Resolution: partial upload remains non-authoritative and recovery returns the complete old input set.
Criterion: Both Sqrzl partitioned-compaction tests are wired into scheduled/manual Cloud Qualification.
Evidence:
.github/workflows/cloud.ymlruns the ignoredcloud_provider_engine_qualificationtarget withsqrzl-tests,failpoints; run 33888223535 executed both named tests successfully, then passed the full all-features suite.Production entry point: the workflow starts the repository's Sqrzl service and runs the provider-facing qualification binary rather than a filesystem substitute.
Resolution: no additional workflow selection is required; the existing command is live and green.
Criterion: Issue perf: bound compaction memory and partition SST outputs #270's local, simulated-cloud, and Sqrzl-backed multi-output upload, publication, and recovery requirement is satisfied.
Evidence: the local and simulated-cloud reopen regressions pass in the all-features workspace suite; the two Sqrzl variants pass in run 33888223535; and the complete-set publication crash proof remains green.
Production entry point: all three modes use
Engine::compact_allplus normal manifest publication and reopen recovery.Resolution: the coverage gap recorded by test: add Sqrzl-backed qualification coverage for partitioned compaction (gap from #270) #272 is stale, and this PR supplies the final closeout evidence.
Criterion: Cloud upload and delete outage regressions fail at provider boundaries regardless of container privileges and cannot contaminate parallel tests.
Evidence: scheduled Docker run 33855817641 showed root could delete from a read-only directory. First-head run 33888223552 proved the repaired delete test passed, then found the same root bypass in
should_handle_cloud_unavailable_during_eviction. Replacing that chmod path with the existing SST-upload hook initially produced a local parallel-suite RED when another hybrid test observed the process-global failpoint. The final design runs that injection in an exact-test child process; the focused test, three consecutive full hybrid targets, and the complete parallel workspace suite pass.Production entry point:
HybridPersistence::write_sst_object_withinanddelete_sst_object_blockingare the actual remote immutable-object provider boundaries used by flush/compaction and GC.Resolution: both tests require the exact provider error and storage outcome; delete workers are joined before observation, and upload injection is confined to a child process.
Criterion: The simulated-cloud partitioned-compaction durability proof remains bounded and reliable on hosted Windows runners.
Evidence: Platform run 33819303183 timed out the real
CompactAllrequest at the fixture's 60-second response limit while the runner also executed the intentionally large resource proof. The focused production-path test completes locally in 39.53 seconds and passes with an explicit 180-second fixture allowance.Production entry point: the fixture still drives the normal runtime request, streaming compaction, manifest publication, and cloud reopen paths.
Resolution: only this qualification fixture's response timeout changes; production defaults and deadline-specific tests remain unchanged.
Red-to-green evidence
Hosted Docker RED: run 33855817641 failed
should_handle_gc_when_cloud_delete_failsbecause its root process defeated the directory-permission delete outage.Local TDD RED: after rewriting the delete test but before adding the boundary hook, its focused command produced 0 passed / 1 failed because no input object remained.
First-head Docker RED: run 33888223552 passed the repaired delete case, then failed
should_handle_cloud_unavailable_during_evictionbecause its sibling permission-based upload outage was also bypassed by root.Local parallel RED: after using the existing upload failpoint directly, the full workspace failed
should_prefer_local_reads_before_evictionbecause that global failpoint leaked to a concurrent test.Final GREEN: provider-boundary upload injection now runs in an exact-test child process. The focused upload case, three consecutive nine-test hybrid runs, and the complete parallel workspace suite all pass.
Hosted Compatibility RED: run 33892098450 passed all four provider-feature jobs, then Rust 1.97 pedantic clippy rejected
Duration::from_secs(180)in favor of the equivalentDuration::from_mins(3)expression.\n- Hosted Core RED: run 33890738899 failed the backpressure OOM qualification because its fixed-delay metric sample raced asynchronous natural-flush publication.\n- Hosted Windows RED: run 33819303183 failedshould_preserve_partitioned_compaction_across_simulated_cloud_reopenwhenCompactAllexceeded the 60-second fixture response timeout.Local Windows-path GREEN: the focused simulated-cloud test passed through real compaction and reopen in 39.53 seconds with the test-only bounded allowance.
Criterion: The backpressure OOM qualification waits for a real pressure outcome instead of sampling asynchronous metrics after a fixed delay.
Evidence: Core run 33890738899 failed on exact prior head
c04701700fdcde9dd0e6439cc523662e3ef36117after all 64 bounded writes completed before natural-flush publication was observable. On the repaired head, five consecutive focused runs, the complete seven-test backpressure target, and the full workspace suite pass.Production entry point: the test commits through the public transaction API and observes runtime write-stall and published-SST metrics.
Resolution: the workload is deterministic and bounded, then waits up to a diagnostic deadline for either a write stall or a published natural flush.
Risk and compatibility
Risk is low. The new delete hook and reused upload hook compile only with the established
failpointsfeature. Process isolation prevents test-global injection from affecting parallel work. The timeout change is scoped to one durability qualification fixture. Public APIs, production timeout defaults, storage formats, manifests, compaction intents, and cloud authority rules are unchanged. Failed cloud deletion remains conservative: obsolete data is retained rather than deleted unsafely.Verification
cargo test --test backpressure --all-features should_prevent_oom_by_rejecting_writes_when_budget_exceeded -- --exact --nocaptureruns - passed, 1 / 1 each.\n-cargo test --test backpressure --all-features -- --nocapture- passed, 7 / 7.\n-rustup run 1.97 cargo clippy --workspace --all-targets --no-default-features -- -D warnings -D clippy::pedantic- passed.\n-cargo test --test engine_gc_cloud--all-features should_handle_gc_when_cloud_delete_fails -- --nocapture` - passed, 1 / 1.cargo test --test engine_gc_cloud --all-features -- --nocapture- passed, 3 / 3.cargo test --test hybrid_storage --all-features should_handle_cloud_unavailable_during_eviction -- --nocapture- passed in its isolated child process.cargo test --test hybrid_storage --all-featuresruns - passed, 9 / 9 each.cargo test --lib --all-features engine::tests::should_preserve_partitioned_compaction_across_simulated_cloud_reopen -- --nocapture- passed, 1 / 1 in 39.53 seconds.cargo test --workspace --all-features- passed; library 1,874 passed / 4 ignored, every integration target passed, and 3 doc tests passed.cargo test --test testing_governance --all-features -- --nocapture- passed, 15 / 15.cargo clippy --workspace --all-targets --all-features -- -D warnings -D clippy::pedantic- passed with zero warnings.cargo check --release --workspace --all-features- passed.cargo fmt --all -- --check- passed.cntryl-tools validate-tests- 2,641 / 2,641 compliant.git diff --checkandgit diff --cached --check- passed.ecfd4efbf2121e38c81a7ac66d7b3b6ef6d5c826.Tool assistance disclosure
OpenAI Codex materially assisted live-state inspection, diagnosis, implementation, test execution, diff review, and PR preparation. Every result claimed above was observed in this checkout or linked hosted run.
Contributor responsibility
Review checklist
tests/coverage_manifests.rs, or no public enum variant was added.