Fix analysis-zip corruption on shared-FS (k8s/NFS) deployments (#857); nginx keepalive/gzip (#864) - #865
Merged
Merged
Conversation
…ct, verified download, receipt re-check, NFS-safe lock probe (#857) Four defects, each with a red-first regression spec: 1. extract_archive (worker + Analysis) used the Zip::File.open block form with ::Zip.sort_entries = true: the implicit close calls commit, which REWRITES the archive in place (temp + rename) for any unsorted zip. Web-background rewrote the live seed_zip at analysis start while workers downloaded it; workers rewrote the shared analysis.zip after every extract. Now uses read-only Zip::File.new (same rationale as seed_zip_error) and drops the global sort_entries. 2. analysis.zip downloads wrote straight to the final shared path unvalidated; truncated bytes were extracted and escalated to terminal CorruptAnalysisZip failures. Now: download to pid-suffixed temp, validate via Analysis.seed_zip_error, atomic rename. Transient corruption retries; 3 consecutive validation failures escalate to the existing terminal CorruptAnalysisZip cleanup (keeps #841 fail-fast for corrupt-at-rest zips). 3. A worker that raced past the outer receipt check re-downloaded and re-extracted even when the receipt appeared before it acquired the lock (N-worker serial re-download herd observed as 6 downloads of one analysis zip in 3s). Now re-checks the receipt while holding the lock. 4. lock_abandoned? probed with LOCK_EX on a read-only fd: legal on local filesystems, Errno::EBADF on NFSv4 (flock emulated as write byte-range lock), crashing every waiter on k8s/NFS - and the deployed EBADF->abandoned runtime patch turned that into live-lock deletion + concurrent re-init on a new inode. Probe now uses LOCK_SH (grantable on read-only fds everywhere, still blocked by a live LOCK_EX) and treats an indeterminate probe as held, never abandoned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
keepalive_timeout 0 (copy-paste from 2016 stock sample, not a tuning choice) forced a new TCP conn per asset/API request; gzip off served HTML/JSON uncompressed. Matches NatLabRockies/openstudio-server-helm c3c91ea ConfigMap override so that helm-side fork can be dropped once images rebuild. Refs #864 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 30, 2026
Closed
overwrite was declared but never checked - existing files were always skipped, so a worker killed mid-extract left stale measure.xml files the next worker kept (BCLMeasure "could not be read as XML data"). Red-first regression specs for both copies; overwrite=false skip path pinned too. Refs #858 Co-Authored-By: Claude Fable 5 <noreply@anthropic.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
develop twin of #863 (which targets
179/ OS 3.10; develop = OS 3.11). Both branches will be maintained for a while, so the fix lands in both. Cherry-picked fromfix/zip-read-only-extractminus the 179-only CI image-push commit.Fixes #857. Closes #864. Closes #858.
Zip corruption fix (#857)
Corruption chain found on the k8s/NFS cluster (flock exclusion itself works there):
lock_abandoned?probedLOCK_EXon a read-only fd —Errno::EBADFon NFSv4; the deployed EBADF→abandoned override then deleted live locks, letting a second worker initialize the same analysis_dir concurrently.extract_archive(worker +Analysis) used theZip::File.openblock form with::Zip.sort_entries = true— implicit close commits, rewriting the archive in place for any unsorted zip (web-background rewrote the live seed zip while workers downloaded it).CorruptAnalysisZip.Changes:
lock_abandoned?: probe withLOCK_SH(grantable on read-only fds everywhere, still blocked by a liveLOCK_EX); indeterminate = held, never abandoned.extract_archive(both copies): read-onlyZip::File.new, no globalsort_entries— archive byte-identical after extract.Analysis.seed_zip_errorvalidation → atomic rename; 3 consecutive validation failures escalate to existing terminal cleanup (Enhance InitializeAnalysis resilience for corrupt seed.zip files #841 preserved).Regression specs written red-first against unfixed code; 36 examples green.
overwrite fix (#858)
extract_archive(worker +Analysis) declared anoverwriteparameter but never checked it — existing files were always skipped, so a worker killed mid-extract left stalemeasure.xmlfiles the next worker kept. Now honored (default true);overwrite=falseskip path pinned by spec. Red-first regression specs both copies.nginx fix (#864)
docker/server/nginx.conf:keepalive_timeout 0→65,gzip off→ on (level 6, standard types, min 1000B). The 0/off values were copy-paste from the 2016 stock sample, not tuning. Matches the NatLabRockies/openstudio-server-helm c3c91ea ConfigMap override so that helm-side config fork (which hard-pins the passenger 6.0.27 path) can be dropped once images rebuild.🤖 Generated with Claude Code