Repository navigation
test: make recovery fixtures native-page-size aware - #65
Open
ziqifan617 wants to merge 1 commit into
Open
ziqifan617 wants to merge 1 commit into
ziqifan617 wants to merge 1 commit into
Conversation
Size the guarded-startup DRAM fixture from its block size so 64 KiB-page hosts reach the intended G3 validation failure. Compare snapshot telemetry with the actual mapped region rather than counting alignment padding. Exercise small, aligned, and unaligned pools through recovery and cleanup. Validated on aarch64 Linux with 64 KiB pages: baseline 432 passed / 2 failed; patched 436 passed. Production code is unchanged. Assisted-by: OpenAI Codex Signed-off-by: Ziqi Fan <ziqif@nvidia.com>
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.
Summary
Fix two unit-test assumptions that fail on Linux hosts with 64 KiB pages, and extend the recovery test to exercise alignment explicitly. No production code changes.
Root causes and changes
test_a_guarded_startup_that_fails_gives_back_everything_it_took[g3-invalid]chooses a block size ofmmap.PAGESIZE // 2, but its fake DRAM pool was fixed at 8 KiB. On a 64 KiB-page host, the 32 KiB block cannot fit, so startup fails before reaching the intended G3 validation/cleanup path. Size the fixture to hold two blocks and use the same block-size variable in the configuration.test_a_handback_region_lives_and_dies_inside_the_pool_filecompared snapshot telemetry against file size minus pool mapping size. That includes alignment padding: a 12,288-byte pool on a 64 KiB-page host introduces a 53,248-byte gap before the snapshot. Compare against the actual mapped snapshot length instead.Validation
Previously executed on native GB300 Linux aarch64, 64 KiB pages, Python 3.12.3, pytest 8.4.2, NIXL 1.5.0. Baseline is
fb4f2643fff47cffe0b21266e170f5569605f9b2; tested commit is98242c1bb835ef68342478354f9a9b44402f3ba7.The increase from 434 to 436 total tests comes from the two additional pool-size parameter cases. Both original failures are reproduced on the baseline and pass with the changes. No tests were skipped or marked xfail.
python -m pytest -q tests/unit: 436 passed (87.15 s).python -m ruff check src tests: passed.python -m ruff format --check tests/unit/test_kvcr.py tests/unit/test_recovery_journal.py: passed.git diff --check: passed.The baseline and patched suites ran with GPUs hidden and isolated test dependencies. Both used a real parent process for pytest: invoking pytest directly through
kubectl execinitially exposed an unrelated PPID-0 assumption in a pidfd test. Using the same parent-process wrapper for both runs restored the expected two-failure baseline; no unrelated test was changed. This is native host unit-test validation, not a new model-serving benchmark. A 4 KiB-page Linux run has not been performed for this change.Implementation and validation were assisted by OpenAI Codex.