Preserve stable params lock ownership - #116
Merged
Merged
Conversation
FrogAi
force-pushed
the
codex/stabilize-params-locking
branch
from
August 10, 2026 02:58
edaa76f to
4a8942f
Compare
FrogAi
force-pushed
the
codex/stabilize-params-locking
branch
from
September 4, 2026 21:28
4a8942f to
28621ff
Compare
FrogAi
added a commit
to FrogAi/mapd
that referenced
this pull request
Sep 4, 2026
Retain the original PR commits and the tested rewrite. The resulting file tree is identical to 28621ff.
Replace the earlier implementation with the simplified version.
FrogAi
force-pushed
the
codex/stabilize-params-locking
branch
from
September 4, 2026 21:50
ea137ed to
6034c72
Compare
This was referenced Sep 5, 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.
Keep the shared params
.lockinode in place after writes and removals. Unlinking that pathname lets a second process acquire a new lock file while an existing process still owns the old inode, defeating cross-process exclusion.Close owned value/directory handles, preserve the existing lock permissions and bounded acquisition policy, and report meaningful close errors.
PutParamcloses and checks its temporary file before publication. Both operations close the directory after syncing it and return a close error when no earlier sync error takes precedence.The close-error evidence models Go API return values after a real close. It does not demonstrate corrupt replacement data, physical disk failure, crash durability, or device incidence.
Ownership, error ordering and complete fault reproducer
The successful write sequence is Write → temporary Sync → temporary Close → lock → Rename → directory Sync → directory Close → unlock. Temporary-file cleanup remains deferred for earlier Write/Sync errors; ownership clears immediately after explicit Close even when it reports an error. Directory handles have only Sync then Close before return, so direct calls and preserved error precedence suffice without extra deferred ownership state. No raw close syscall is retried.
The lock is created with mode 0775 subject to umask; existing mode/inode are retained. Acquisition remains at most 51 attempts separated by 50 one-millisecond sleeps, not a wall-clock guarantee.
RemoveParamignores the unlink result.Tested source and validation scope
7201c6b4b4ec1b0b9ea21daa8c05b80fdd7e01ee.6fd5bbd, treef56b0d35a2f2ab997f7c0b89f7f851a0818cf454.Checks described here used Go 1.25.1 on Linux amd64 with isolated fixtures and networking disabled during behavior tests. Existing repository tests remain unchanged; the extra reproductions below are deliberately outside the committed source. These are local execution results, not physical-device validation. The PR's Checks tab provides the published workflow result.
Linux tests cover binary/empty persistence, real subprocess contention, stable inode, new/existing permissions, GC-disabled descriptor counts, lock-open/rename failures, and temporary Write/Sync plus directory Sync errors. Normal/lifecycle race checks, vet and repository tests pass. The portable check below passes all three modeled close-error cases. Additional cleanup injection confirms earlier ENOSPC survives a cleanup Close EIO, with no temporary pathname/owned-descriptor leak. No throughput claim follows from cleanup changes.
Use Linux Go 1.25.1, Python 3 and cached dependencies from the PR head listed above. Save this complete script outside the checkout as
reproduce-close.py, then runpython3 /path/to/reproduce-close.pyfrom the checkout. It creates the Go overlay, fixtures and generated caches in its own temporary directory, preserves the selected cached module input, and exercises the real unmodified params source.Combined validation: exact source tree
a4c306906627db3ac7a8ab768651c8628d55465acombines #1019d61f06a1288ec4ea6f74f7d56a3057444316a13, #10320e7c25b054b6399360676a7f539a39b4fbf855c, #105bfcfe77be066634e36054327b20cfa6541063b54, #1078e5e677d1196838069e9665d4e9d962bcc1e116b, #1166fd5bbd6cf617c24a7fefd5e302fd36688a1a63b, #13630e8ce98ea7a4c8401dbb5bfc62120c84fc689e4. The only overlapping file issettings/download.go; the resolution retains #136's selected-row loop and #107's progress publication inside it.Combined Linux amd64 tests (including the scratch regression fixtures), race checks, vet and build passed. Under ARM64 emulation, the existing Makefile build stage (
make GO_CAPNP_PATH=/usr/local/go-capnp/std), committed repository tests, vet and both CLI help commands passed with Go1.25.1;go.mod/go.sumstayed unchanged and the resulting executable is AArch64. The ARM64 run does not include the extra amd64 scratch tests. It used an isolated retained build image, not a new dependency-install/image rebuild or physical device. No archive payload or live params were accessed. #105 still requires runtime-first rollout before regenerated tiles are distributed.