Locking storage - #798
Draft
butonic wants to merge 7 commits into
Draft
Conversation
Introduce a leaf locks package (MemoryLocker, DiskLocker) and a LockingStorage capability interface that embeds Storage. Convert the storage constructors to variadic options so a locker can be injected; Disk defaults to a DiskLocker rooted at its data dir and CS3 to a MemoryLocker. No behavior change yet: UploadWithLock is not implemented.
Add UploadWithLock to the disk backend: it holds a lock on the logical path across read->fn->write so the operation is fully atomic. fn returns nil to abort without writing. Switch disk etags to be content-based (md5) so they act as reliable cache-invalidation tokens. Add tests covering create-only, abort-on-nil, and concurrent counter with both lockers.
Add UploadWithLock to the CS3 backend: it acquires a lock on the logical path, downloads the existing content (nil if absent), invokes fn, and uploads the result without etag preconditions since the lock guarantees exclusivity. Returns a fresh etag for cache invalidation. Add tests for lock serialization and concurrent same-key updates.
Convert the provider, user-share, and received-share caches from etag-based compare-and-swap retry loops to a single atomic storage.UploadWithLock read-modify-write. Caches now hold a metadata.LockingStorage and refresh their in-memory state in place so callers holding references observe updates. Add concurrent-add tests proving no lost updates per cache.
Register a new 'json' share-manager driver backed by the disk storage (with an injectable locker) alongside the existing CS3-backed 'jsoncs3' driver. Add a Dump method to the Manager that walks all three caches and exports shares/received-shares, enabling migration from CS3 to disk. Add tests for dump export and concurrent-add atomicity on disk.
Add an Update method to the Persistence interface performing a single atomic read-modify-write. The file backend uses a DiskLocker sidecar and renameio; the CS3 backend uses LockingStorage.UploadWithLock on publicshares.json; the memory backend uses an in-process mutex. Replace the whole-file Read->mutate->Write in CreatePublicShare, UpdatePublicShare and revokePublicShare with persistence.Update, eliminating the lost-update race. Add a lock_backend config option and a concurrent create/revoke test.
Add an optional LockingStorage to metadatacache.Options. When set, Store.Update performs a single atomic UploadWithLock instead of the etag-based CAS retry loop; otherwise the existing CAS path is unchanged. Wire the CS3 storage as a LockingStorage in the appauth jsoncs3 manager and add a lock_backend config option. Add tests covering the locking path including concurrent same-key updates with no lost updates.
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.
This PR serves as a PoC to make share persistence fully
atomic by replacing etag-based optimistic
locking (compare-and-swap + retry loops) with pessimistic, locked
read-modify-write operations. This removes the fragile CAS retry machinery from the
jsoncs3 share caches (and appauth store) and fixes real lost-update races in the
publicshare manager. It also introduces a new disk-backed
jsonshare-manager driver as an alternative to the CS3-backedjsoncs3driver, with an in-place migration path between them.
The short version is that I extended the old Storage interface with an UpdateWithLock method and used an Option pattern to inject a memory or disk based lock implementation (mimicking dcomposedfs metadata access).
Why
The jsoncs3 share manager is known not to scale under load:
The retry/CAS approach was patched by opencloud incrementally over time to cope with
transient storage errors and cross-user share removal:
Rather than keep hardening a fundamentally racy pattern, this PR removes the
CAS loops entirely and makes each write a single atomic locked operation.
What changed (7 commits)
metadata/lockspackage (MemoryLocker,DiskLocker) +LockingStoragecapability interface; storage constructors become variadic(no behavior change).
Disk.UploadWithLock: lock held across read→fn→write; disk etagsbecome content-based (md5) so they act as reliable cache-invalidation tokens.
CS3.UploadWithLock: same atomic semantics over the CS3 provider.loops to one
UploadWithLock; in-memory caches refresh in place.jsonshare-manager driver + addDump(migration: dump from CS3 → load into disk).
Updateacross all three persistencebackends; fixes the lost-update race in create/update/revoke.
metadatacache.Store: optionalLockingStoragepath(atomic) with the etag-CAS loop kept as fallback.
Configuration
New
lock_backendoption (diskdefault, ormemory) on the publicshare andappauth jsoncs3 managers. Disk locks use
<key>.mlocksidecar files, somulti-replica deployments need a shared (RWX) volume at the data root.
Testing
updates), run with
-race.