Skip to content

posixage: use caller context for lock waits#523

Merged
Benehiko merged 1 commit into
docker:mainfrom
ilopezluna:configurable-posixage-lock-timeouts
May 26, 2026
Merged

posixage: use caller context for lock waits#523
Benehiko merged 1 commit into
docker:mainfrom
ilopezluna:configurable-posixage-lock-timeouts

Conversation

@ilopezluna
Copy link
Copy Markdown
Contributor

@ilopezluna ilopezluna commented May 21, 2026

  • use the caller context to bound lock acquisition instead of the previous internal 100ms wait
  • keep stale lock recovery internal with the existing fixed 30s age threshold
  • update posixage flock tests and README locking docs for context-driven waits

Verification:

  • go test ./store/posixage/...
  • git diff --check

@ilopezluna ilopezluna force-pushed the configurable-posixage-lock-timeouts branch from e1655c6 to 06a3dbd Compare May 21, 2026 18:57
@ilopezluna ilopezluna marked this pull request as ready for review May 22, 2026 09:16
Copy link
Copy Markdown
Member

@Benehiko Benehiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The capability is right, but the API shape needs rework before merging.

Drop WithLockTimeout and flock.Config.LockTimeout. context.Context already expresses "how long to wait" — callers who want a bounded wait pass context.WithTimeout; callers who want no timeout pass a ctx without a deadline. That gives you the disable-timeout case for free, no new option needed.

Keep WithStaleLockTimeout only. It's a file-age threshold, not a wait duration — semantically distinct, belongs as a store-level option.

Move stale recovery trigger off LockTimeout. With LockTimeout gone, trigger recovery on first-attempt failure instead (try once → on fail, attempt recovery if StaleLockTimeout > 0 and ctx not canceled → retry until ctx done).

Proposed signature:

func TryLock(ctx context.Context, root *os.Root, staleAfter time.Duration) (UnlockFunc, error)

Defaults stay (no deadline = wait forever; 30s stale). Drops one option, one struct, and the LockTimeout/StaleLockTimeout coupling.

store/posixage/internal/flock/flock.go:

  • tryLock — drop cfg Config param, take staleAfter time.Duration; restructure to "first-attempt → recover → retry til ctx done"
  • retryLock — drop timeout param, retry purely against ctx
  • TryLock / TryRLock — new signatures (ctx, root, staleAfter)
  • Delete Config, DefaultConfig, defaultLockTimeout const

store/posixage/store.go:

  • fileStore.tryLock / tryRLock — pass f.staleLockTimeout instead of f.lockConfig
  • config struct — replace lockConfig flock.Config with staleLockTimeout time.Duration
  • Delete WithLockTimeout
  • WithStaleLockTimeout — stays, writes to new field
  • New — init default staleLockTimeout = 30 * time.Second (move const from flock pkg or expose getter)

store/posixage/internal/flock/flock_test.go:

  • All tests passing Config{...} — rewrite to pass staleAfter directly; tests covering LockTimeout behavior become ctx-deadline
    tests

store/posixage/store_test.go:

  • Any test exercising WithLockTimeout — remove or convert to ctx-deadline

store/posixage/README.md:

  • Update the new lock-config section

Comment thread store/posixage/internal/flock/flock.go Outdated
@ilopezluna ilopezluna changed the title posixage: make lock timeouts configurable posixage: make stale lock timeout configurable May 26, 2026
@ilopezluna ilopezluna force-pushed the configurable-posixage-lock-timeouts branch from 1bbb7c6 to 0bdc29a Compare May 26, 2026 10:38
@ilopezluna ilopezluna changed the title posixage: make stale lock timeout configurable posixage: use caller context for lock waits May 26, 2026
@Benehiko Benehiko merged commit 31241f3 into docker:main May 26, 2026
7 of 15 checks passed
@Benehiko Benehiko linked an issue May 26, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Simplify posixage lock acquisition API

2 participants