Repository navigation
Conversation
|
One concern about the default path. The check is on whenever |
Resolve conflicts by keeping restore_has_headroom OOM admission with upstream spread_victim/borrow conversation-cache changes.
|
Thanks for the pull request. We're resetting the tracker for 0.2.0-beta. There are more than 1,300 open issues and pull requests, too many for us to review properly, so we're closing everything that isn't being worked on right now. Closed here does not mean rejected. To reopen it takes about two minutes:
The rules are in CONTRIBUTING.md. |
|
/reopen |
|
Not reopened yet. Still missing:
Edit the pull request and comment |
|
/reopen |
|
Reopened, thank you. A maintainer will pick it up from here. If it conflicts with main, rebase it first; that speeds up review. |
…ate its copy stream and has no room for it (#2409) Problem. #2408 / PR #2409: restoring a large parked conversation on a layer split ended the server with a CUDA out of memory on a later card. The PR checks the free VRAM of every card against --vram-reserve-mib before a restore. Its own review points out that a loaded card is always below that reserve, so the check would skip nearly every restore on a split (and on one GPU). It is not ported. What a restore allocates. conversation_snapshot_restore writes into the session's existing buffers (pools(st), the GDN state, the indexer tails); refresh_residency only launches kv_stream_reset or copies into the existing ring. The elastic K/V growth (kvg_ensure) allocates, but only on one GPU. The one thing a restore can create on a card is the calling thread's private copy stream for it (conversation_copy.hpp private_stream), which it takes from the pool of finished threads when there is one (#3350). A failed restore is fatal by contract (the session may be half written). So the need is one stream and one event, and only for a card whose stream does not exist yet (a snapshot loaded from disk, or a restore on a thread that never copied on that card). Change. conv_cache.hpp: kRestoreFirstUseMib (64) and restore_fits(free, stream_ready, n, need): a restore is refused only for a card that has no copy stream yet and less than 64 MiB free. conversation_copy_stream_ready(device) says whether a card has one. generate.cpp: before the restore, each card of the run is checked; a snapshot that would not fit goes back to the cache and the prompt is read again, with a log line. A card that has its stream is never refused, however full. No sample (a failed cudaMemGetInfo) means the restore goes ahead as it did. Tests. conv_cache_test: the predicate (cards with a stream are admitted at any free size; the edge at the need; no sample / no cards / no need). conversation_copy_streams_test: copy_stream_ready before and after the first copy, in the pool after a thread ends, and the measured cost of a card's first copy stream and event (printed; the test requires under 16 MiB, so 64 MiB leaves a wide margin). conversation_snapshot_test: after the first restore, six more restores of two images leave the free VRAM where it was. Not covered. The pure predicate and the generate.cpp code build on z440 (CUDA 13.0); the GPU tests are listed in the stream notes. No model run: gates/2409-restore-guard.sh (a held-VRAM start on a layer split) shows the skip line and an unchanged answer on the second turn.
P0 #3911 #1503; P1 fixes; file-02 items; 22 PR ports (authors kept) incl. #3779 (sm_80 defaults under STRATA_DEFAULTS=026) and #4121 (STRATA_PIPED_MTP opt-in); audit fixes #3602 #3948 #3638 #3797 #2409; owner decisions #3935 (DeepSeek sm_89 packages), #3844, #3453 (sm_89 tables on), #3976 (layer-major prefill on by default for a 2-card CUDA layer split; revert 1e41fa269 to undo). Final gate: sm_60, SYCL, z440 sm_86 CPU 38/38 GPU 23/23, boston CPU 24/24 GPU 23/23, owner-PC MSVC CPU 36/36 GPU 20/20, snapshot 10/10 on z440 and boston.
|
Thank you. Fixed in 0.2.7: Skip a restore only when a card would have to create its copy stream and has no room for it. We could check it with unit tests and builds, but not on your hardware or with your workload. Please try 0.2.7 and tell us how it goes. Release: https://github.com/Niko1221/Strata/releases/tag/v0.2.7 |
- Niko1221#3602: a failed PLE table read no longer leaves the next ticket waiting forever; upstream's ple_reader_stress_test added (hangs on the old reader, passes now). - Niko1221#3350: a conversation checkpoint whose copy runs out of memory is skipped instead of ending the engine; STRATA_TEST_COPY_FAIL_AT=N hook. - Niko1221#2409: no restore on a card without its copy stream and under 64 MiB free; the snapshot goes back to the cache and the prompt is reread. Checks this thread's stream only (no stream pool in this tree). - Niko1221#3796: a declared mcp__a__b written as mcp_a_b is that tool; strata_rejected_tool_calls marker and tool_calls_unread metric. The local rule that undeclared calls stay calls when tools are declared is kept. Engine 0.1.44.1+0.2.7-fixes (gfx1102). Smoke-tested; no benchmark. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ate its copy stream and has no room for it (Niko1221#2409) Problem. Niko1221#2408 / PR Niko1221#2409: restoring a large parked conversation on a layer split ended the server with a CUDA out of memory on a later card. The PR checks the free VRAM of every card against --vram-reserve-mib before a restore. Its own review points out that a loaded card is always below that reserve, so the check would skip nearly every restore on a split (and on one GPU). It is not ported. What a restore allocates. conversation_snapshot_restore writes into the session's existing buffers (pools(st), the GDN state, the indexer tails); refresh_residency only launches kv_stream_reset or copies into the existing ring. The elastic K/V growth (kvg_ensure) allocates, but only on one GPU. The one thing a restore can create on a card is the calling thread's private copy stream for it (conversation_copy.hpp private_stream), which it takes from the pool of finished threads when there is one (Niko1221#3350). A failed restore is fatal by contract (the session may be half written). So the need is one stream and one event, and only for a card whose stream does not exist yet (a snapshot loaded from disk, or a restore on a thread that never copied on that card). Change. conv_cache.hpp: kRestoreFirstUseMib (64) and restore_fits(free, stream_ready, n, need): a restore is refused only for a card that has no copy stream yet and less than 64 MiB free. conversation_copy_stream_ready(device) says whether a card has one. generate.cpp: before the restore, each card of the run is checked; a snapshot that would not fit goes back to the cache and the prompt is read again, with a log line. A card that has its stream is never refused, however full. No sample (a failed cudaMemGetInfo) means the restore goes ahead as it did. Tests. conv_cache_test: the predicate (cards with a stream are admitted at any free size; the edge at the need; no sample / no cards / no need). conversation_copy_streams_test: copy_stream_ready before and after the first copy, in the pool after a thread ends, and the measured cost of a card's first copy stream and event (printed; the test requires under 16 MiB, so 64 MiB leaves a wide margin). conversation_snapshot_test: after the first restore, six more restores of two images leave the free VRAM where it was. Not covered. The pure predicate and the generate.cpp code build on z440 (CUDA 13.0); the GPU tests are listed in the stream notes. No model run: gates/2409-restore-guard.sh (a held-VRAM start on a layer split) shows the skip line and an unchanged answer on the second turn. (cherry picked from commit 8c03014)
|
Thank you! Worked perfect |
Re-applies the generate.cpp part of 1aa35f0 ('conversation cache: park and restore conversations through session files') onto the post-v0.2.7 generate.cpp, with the 8b14892 banner change. --conversation-cache-disk-mib N parks switched-away conversations in session files under <pack>/conversation_cache (or --conversation-cache-dir DIR); --conversation-cache-days D is the age a file is kept (0 empties the folder at startup). Default off: the RAM path is unchanged. Options, usage, parser (next to the other --conversation-cache-* options): --conversation-cache-disk-mib, --conversation-cache-days, --conversation-cache-dir. Setup: - conv_disk = disk MiB > 0 and no layer split and no multi-GPU. Otherwise the flag is refused with a message. --conversation-cache-mib is ignored with a notice while disk is on. - The Niko1221#3493 RAM auto-size is skipped in disk mode (the RAM budget is not used). - The ConversationCache is built with the disk budget and enable_disk(). - Startup scan (after session_identity): conversation_cache_index() adopts the previous run's files oldest first as slim index entries via put_disk(); files that cannot be indexed are removed. A failed scan turns disk mode off and rebuilds the RAM cache, only when disk was on. Park (park_source, the shared park for the main session and for batch slots): - after drop_superseded, a disk branch: only the deepest checkpoint goes to the file; the K/V streams from the pools (conversation_snapshot_sources, pointer draft form, so a session without --mtp works); the snapshot must fit the disk budget and pass the RAM admission gate; the write is durable=false with the watchdog's progress and phase hooks; put_disk(conversation_meta_only(meta)). - every refusal is the same skip-park success the RAM path gives. - Adaptation: the pre-merge park_source had the same shared shape, so slot parks also go to disk here, which the pre-merge code did too. Restore: - a disk entry is read with session_file_read under the RAM gate (conversation_session_read_limits, pointer form) and never borrowed. The entry leaves with remove_at() once the read worked; the file goes after the state is on the device (log: "took back N tokens from disk"). - a read that fails on the file (invalid/io) removes the file; a transient refusal (limits, RAM gate) keeps it for the next start's scan. - an unused read is dropped with its file. - Adaptation: the restore_fits retry (Niko1221#2409, upstream, not in the pre-merge tree) put the decoded image back into RAM with conversations.put(). Disk mode now puts it back as its file (put_disk), so the disk budget and RAM stay honest. - no RAM K/V retention (retain) in disk mode (the file is the K/V). Exit: a clean exit (QUIT or stdin EOF) parks the live conversation to disk, so the next start's scan finds it (nan-poisoned sessions are not parked). Banner (8b14892, disk part): the conversation_cache_mib= log field shows the disk MiB in disk mode. Not ported, with the reason: - 8b14892 DFlash2 context park/restore: out of scope. - 8b14892 take_superseded_reuse guard (and the `!conv_disk` harvest gate): the function (36b01d3) is absent from the post-v0.2.7 tree, so there is nothing to guard. The drop_superseded path already unlinks (1aa35f0 rule). - 5a14ef2's generate.cpp SAVE/RESTORE session_draft hunk: the disk cache does not need it (it calls the pointer overloads directly). Still open: the SAVE and RESTORE commands pass mtp.kv_state() unconditionally. - 69fb791's generate.cpp RESTORE streamed path and its snapshot-level binding (ConversationKvTarget, conversation_kv_targets): not needed. A disk restore reads the whole file, as the pre-merge disk path did. - 94f55cd's generate.cpp layer-split SAVE/RESTORE (draft_stage_kv, kv_stages, conversation_stage_read_limits): not needed, since disk mode refuses a layer split and multi-GPU. Verified (host, no GPU): src/program/generate.cpp -fsyntax-only with the parent's flags.make flags (worktree include first): 0 errors; 137 warnings, identical to the base tree (130 -Wunused-result, 4 -Wunused-variable). Not exercised at runtime: the park/restore path needs the GPU engine. (cherry picked from commit 4ae761b3bee4dedb0c09bdd96498fb801f8a9b5c)
…RestoreResult The 27/29 Oct merges moved the enum into the Niko1221#2409 helper's neighborhood (line 218), below the fork's grafted ConversationKvRestoreResult, whose default member initializer names ConversationRestore::invalid - the header could not compile by itself. The enum now sits with its doc comment before the struct, as in fork parent 72cacf6; a pointer comment stays where the merge had put it. No duplicate definitions.
…ion at 0.1.40.1) dpct 2025.3 of the 0.1.40.1 tree (23a38a0) and of 870aedd, normalize.sh, a 3-way merge into sycl/ per docs/INTEL.md (src/dsv41 left out: b70 keeps its own DeepSeek port). Upstream's history restarts at 424b364, whose SYCL copies were still at 0.1.40.1, so the base is the last re-migration, not the merge base. 38 merged files conflicted (about 520 hunks), plus the hand-kept copies (verify, mtp, conversation_state, remote/peer experts, direct_file); every conflict resolved with the port's kernels, launch shapes and B70 defaults kept. In (opt-in unless noted): layer-major prefill (STRATA_PREFILL_LAYER_MAJOR, FP16 expert path on SYCL), PLE residency policy (default as upstream: table in RAM only with table + arena + 8 GiB free), PLE_COMPACT / PLE_COPY_STREAM, DEQUANT_SIDE, KVA, SELECT_LONG (multi-group decode top-k + dx scorer), SELECT_SIMT, QSA_ROWS / slots, SPEC_COST, SPEC_PROB, PIPED_MTP, logit bias / allowed_token_ids / logprobs in the sampler, conversation cache (PARK_ASYNC, CKPT_ASYNC, CKPT_POOL_READY, PARK_VRAM_MIB, Niko1221#1503 Niko1221#2409 Niko1221#3350 Niko1221#4420), expert source file-tier prefetch (IO_PF_*, FS_*), RAM_ADAPT, copy_blobs, graph eviction on out-of-memory (Niko1221#3702), stager waits and copy-wait dedupe (Niko1221#4005), PLE reader fix (Niko1221#3602), cache reserve (Niko1221#4474), QFUSE commit fix (Niko1221#3948), 9-16 token windows (capability; kVerifyMaxT stays 8), Gyro-L TQP5/6/7, EXPDEC, IQ_STAGE_TABLES, GU_SWIGLU, HC_PERSIST, GR_UP_WIDE, B6_MMVQ_ROWS, ROUTE_PRIOR (in strata_kernels now), LOGITS_DUMP, NAN_GUARD, TRACE. Off on SYCL: PDL (pdl.hpp: no Level Zero counterpart), PF_WMMA (gemm_iq_f16_grouped stub), PATTN_SM60, the sm_60..121 / gfx / GP100 tables and kernels, NVML, DFlash2 (--draft dflash2 refused), NVFP4, Qwen3.6, argmax_rows (a dpct major version is not a compute capability), route tail skip default (decode_defaults_arch_ok false). Upstream headers b70 had removed are back in include/strata (host-only). Checked: every changed translation unit compiles (JIT), strata links.
…/8c030148/ebe3383a/894f133b) - Niko1221#1503 conv-cache: retain() records the conversation's ids; park takes take_reuse_for(live) (prefix-match only, never another conversation's K/V); copy_from_slot drops the retained image. Our struct had no ids field: the leak was live-shaped (batch 6 slots + parking, 262K window). - Niko1221#2526 park_slot: park_current_body -> park_source(ParkSource) + park_slot(b) before copy_to_slot at admissions and BYIELD reads (an idle slot's decoded reply was re-read whole on the next turn); slot checkpoints come along at admission (drop_superseded applies). Adapted: no cross-thread stream pool in our tree; checkpoint_failure_is_survivable() dropped. - Niko1221#2409 restore guard: restore_fits/kRestoreFirstUseMib in program/conv_cache.hpp + conversation_copy_stream_ready (thread-local stream check, adapted from upstream's pool) + the pre-restore skip in the serve path (snapshot returns to the cache, prompt re-read). - Niko1221#3602 PLE reader: a failed read no longer leaves the next ticket waiting forever (issue() returns a jobless ticket once failed; collect() drops dead tickets). - Niko1221#3465 NaN retry: non-finite logits in a PROMPT window -> the retry reads the whole prompt batched (short_read off for one request).
Fixes #2408
Problem and result
On a two-GPU layer split, restoring a large conversation-cache snapshot can reconstruct K/V residency on a later GPU that is already below the configured VRAM reserve and terminate the server with CUDA OOM. This change admits a parked snapshot only when every sampled split device has the configured headroom; otherwise it returns the snapshot to the conversation cache and rereads the prompt. A failed or unavailable VRAM sample preserves the existing behavior. Single-GPU restores keep the existing path.
Approach
restore_has_headroompredicate ininclude/strata/program/conv_cache.hpp.--vram-reserve-mib.Validation
b116fd8f42f48e40156c82368e56a8431fee77ac:cmake -S C:/tmp/strata-pr-repo -B C:/tmp/strata-pr-repo/build-cpu -G Ninja -DSTRATA_ENABLE_CUDA=OFF -DSTRATA_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Debugcmake --build C:/tmp/strata-pr-repo/build-cpu --target split_rules_test conv_cache_test --parallel 4— passed.ctest --test-dir C:/tmp/strata-pr-repo/build-cpu -C Debug --output-on-failure -R "^(split_rules_test|conv_cache_test)$"— 2/2 passed.git diff --check— passed.conv_cache_testincludes the below-reserve single-GPU regression case.main/0.2.0-beta and current CUDA, HIP, and SYCL backend builds; these are the remaining checks after the tracker reset.Limits
This is a conservative restore-admission fallback. It does not guarantee cache reuse when split-device headroom is unavailable, and it does not change single-GPU restore admission.