[pull] main from ClickHouse:main - #8
Merged
Merged
Conversation
…les atomically (#44) * fix: report session-workspace failures faithfully and prime inline files atomically Three fixes to how session mode reports and protects workspace state, all found reviewing the subtree import of this code into a downstream monorepo. Inline priming destroyed before it wrote. writeFile unlinked the destination and then wrote the replacement, so a write that failed partway (ENOSPC/EIO) left the previous turn's bytes gone and the workspace dirty — forcing a recycle/restore that loses warm state when no checkpoint exists yet. By-reference priming already avoids exactly this: it keeps a regular file in place and lets a rename replace it atomically. Inline priming now does the same, and still clears a squatting symlink or directory so a prior turn cannot redirect the write. Checkpoint/restore collapsed two different bind failures into one generic 409. A missing header (a caller error) and a REJECTED bind (this runner is pinned to a different session, so it must be recycled) both answered "Missing runtime session header" with no error code, leaving the control plane unable to see the conflict. The conflict case now returns the same session_workspace_dirty signal /execute already uses for this condition, so an older service fronting a newer runner still recycles the VM. A malformed or duplicated header now returns 400 instead of propagating SessionWorkspaceBindingError out of the route. A validation failure after priming reported the workspace as dirty. Any error thrown once priming completed took the dirty branch, so a ValidationError — a deterministic rejection of the request, after which nothing ran — answered session_workspace_dirty instead of 400. That cost a needless restore and hid the reason the caller needed to fix its request. Each fix is covered by a test that fails without it. * fix: reject unrunnable requests before priming instead of after Addresses review feedback on the previous commit. Rejecting a request with no runnable source AFTER priming is too late, and answering it with a clean 400 was worse than the behavior it replaced. `getJob`'s gate accepted any utf8 file, including the `.dirkeep` sentinel, while `Job.execute` required a utf8 file that is NOT `.dirkeep`. A request carrying only `.dirkeep` plus binary inputs therefore passed the gate, reached `prime()` — replacing files in the session workspace and recording priming metadata — and only then failed. Session cleanup deliberately preserves the workspace, so the rejected request's writes stayed visible to the next execution. The previous commit made that case return 400 by moving the ValidationError branch above the dirty branch, which traded a false-dirty for a false-clean: the workspace really had been written to. Both checks now call a single `hasRunnableSource` predicate, and the request gate runs it before any Job is built, so such a request is rejected without touching the workspace. The catch block is restored to its original order: once priming has completed, any later failure — validation included — reports the workspace as dirty, which is the honest answer. Tests: a request with nothing runnable is rejected with prime() never called, and a genuine post-prime failure still reports dirty. Both fail without this change.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )