Repository navigation
P2-A: Prevent lost updates during concurrent Hugging Face dataset exports #193
Description
Context
PR #191 (feat/training-integrity-ledger, audited at head 75df249dcbb36303dfe0cdfdbe86cd7445bbbf8f) reads the existing remote dataset, builds a combined snapshot, and pushes the resulting dataset. The local _flush_lock only coordinates threads within one process.
Risk
Two processes or operators can read the same remote snapshot, independently append different rows, and then overwrite one another's updates when pushing their respective full-dataset snapshots. Manual or external remote edits during the read-to-push window can create the same lost-update risk.
Scope
Follow-up only; do not expand PR #191.
Acceptance criteria
- Document the supported writer model and explicitly state whether only one writer/process is supported.
- Prefer a remote revision/commit precondition or another compare-and-swap strategy where the hosting API supports it; otherwise fail closed when the remote revision changes between baseline read and write, or provide a clearly enforced single-writer mechanism.
- Add tests that simulate two writers or a remote revision change between read and push; prove the second writer cannot silently overwrite unseen changes.
- Preserve fail-closed behavior when the remote revision cannot be verified.
- Keep Hugging Face export disabled by default; do not perform a live upload as part of tests.
- Do not modify
mainuntil separately authorized.
Status
P2 follow-up identified during independent review of PR #191. This is not a merge block for #191 based on the current audit, but it is a real data-integrity risk that must be addressed or explicitly accepted before multi-writer deployment.
Activity
DrAbdulmalek commentedon Oct 12, 2026
P2-A implementation research update (2026-10-12):
The current exporter reads the remote dataset with
load_dataset(...), constructs a full combined snapshot, then callsDataset.push_to_hub(). The process-local_flush_lockdoes not coordinate separate processes/hosts, so this flow has no compare-and-swap precondition today.The Hugging Face Hub API exposes
HfApi.create_commit(..., parent_commit=<sha>); the documented contract says the commit fails if the target revision no longer points to that parent. This is the strongest candidate for remote CAS. Official API docs: https://huggingface.co/docs/huggingface_hub/v0.33.4/package_reference/hf_apiImportant: merely reading
dataset_info().shatwice and then callingpush_to_hub()is NOT sufficient, because a writer can race after the final read. A safe implementation needs a commit path that actually passes the baseline SHA asparent_commit, plus a regression test simulating a stale parent/revision conflict. This may require replacing the current high-level full-dataset push path; do not claim P2-A fixed until that exact path and tests are in place.No live Hub write, merge, or
mainchange was performed.DrAbdulmalek commentedon Oct 12, 2026
P2-A implementation is now proposed in draft PR #195: #195
The writer pins
load_datasetto a verified Hub commit SHA and callsHfApi.create_commit(..., parent_commit=<same SHA>); a stale-parent conflict propagates and is not retried automatically. The implementation deliberately fails closed unless the remote repository has exactly one supported Parquet data file underdata/, because blindly replacing multi-shard or metadata-driven layouts would risk data loss. Added offline tests cover revision pinning, unsupported layouts, exact parent propagation, and stale-parent conflict propagation.No live Hugging Face upload, no merge, and no changes to
main. PR #195 remains Draft pending CI and review.DrAbdulmalek commentedon Oct 12, 2026
Follow-up implementation is in draft PR #196 (stacked on #195): queue append/count/flush now share an OS-backed lock file plus the in-process thread lock, with a multiprocess regression test. This serializes workers sharing the same local queue directory on a filesystem honoring advisory locks; it does not coordinate separate machines. The remote parent-commit CAS remains necessary for cross-machine writers. CI on the latest head is pending; no live Hub write has been performed.