Skip to content

posixage: make lock timeouts configurable#523

Open
ilopezluna wants to merge 1 commit into
docker:mainfrom
ilopezluna:configurable-posixage-lock-timeouts
Open

posixage: make lock timeouts configurable#523
ilopezluna wants to merge 1 commit into
docker:mainfrom
ilopezluna:configurable-posixage-lock-timeouts

Conversation

@ilopezluna
Copy link
Copy Markdown

@ilopezluna ilopezluna commented May 21, 2026

  • add posixage options to configure lock wait and stale lock recovery timeouts
  • preserve the existing 100ms lock wait and 30s stale recovery defaults
  • skip stale lock recovery for context-only waits, disabled stale recovery, or canceled contexts

@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 on lines +58 to +63
func DefaultConfig() Config {
return Config{
LockTimeout: defaultLockTimeout,
StaleLockTimeout: defaultStaleLockTimeout,
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

make this a var, no need to have a function

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.

2 participants