Skip to content

Improve documentation#9

Merged
roxblnfk merged 4 commits into
roadrunner-php:1.xfrom
gam6itko:wait-ttl-comment
Jul 9, 2026
Merged

Improve documentation#9
roxblnfk merged 4 commits into
roadrunner-php:1.xfrom
gam6itko:wait-ttl-comment

Conversation

@gam6itko

@gam6itko gam6itko commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

What

Clarify what the waitTTL parameter of lock() / lockRead() does, in both the docblocks (LockInterface, Lock) and the README.

Why

waitTTL was documented only as "how long to wait to acquire lock until returning false", with nothing about the default value of 0. Worse, the README intro states other processes "will be blocked until the lock is
released"
, which implies the call always blocks — the opposite of the default behavior.

Behavior verified against the server plugin (roadrunner-server/lock, rpc.go):

const defaultImmediateTimeout = time.Millisecond

func waitContext(parent context.Context, waitUs int64) (context.Context, context.CancelFunc) {
    if waitUs == 0 {
        return context.WithTimeout(parent, defaultImmediateTimeout) // 0 → 1ms
    }   
    return context.WithTimeout(parent, time.Microsecond*time.Duration(waitUs))
}

So wait = 0 (the default) is floored to a ~1ms acquire windowthe call is effectively non-blocking and returns false almost immediately when the resource is already locked. A positive wait blocks for up to that duration,
returning the lock id as soon as the lock is released, or false on timeout.

Also confirmed empirically: wait: 0 on a held resource returns false in ~2ms; wait: 1 returns false after ~1.001s.

Changes

- Expand the @param $waitTTL docblock on lock() and lockRead() in LockInterface and Lock.
- Fix the misleading "blocked until released" wording in the class/interface intros.
- Add a note to both README acquire sections describing the non-blocking default and the positive-wait behavior.

Docs onlyno code/behavior changes.

Note: the head spec in the URL is `gam6itko:rr-php-lock:wait-ttl-comment` because your fork is named `rr-php-lock` while upstream is `lock`.

Want me to also install nothing/leave it here, or move on to drafting the rate-limit fix plan for be-pub in `.claude/plan/`?


| Q             | A
| ------------- | ---
| Bugfix?       | no
| Breaks BC?    | no
| New feature?  |  no
| Issues        | #... <!-- prefix each issue number with "#" symbol, no need to create an issue if none exist, explain below instead -->
| Docs PR       | spiral/docs#... <!-- prefix each issue number with "spiral/docs#", required only for new features -->

<!-- Please, replace this notice by a short description of your feature/bugfix.
This will help people understand your PR and can be used as a start for the documentation. -->


<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

* **Documentation**
  * Clarified default lock acquisition behavior, including that exclusive `lock()` is non-blocking by default and returns `false` when the lock can’t be acquired immediately.
  * Documented `waitTTL` semantics for both exclusive and shared locks, including RoadRunner’s capped near-immediate timeout window and `false` on timeout.
  * Updated examples and the lock parameters guidance to use `waitTTL` for read-lock waiting and to better explain return values.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

The `waitTTL` parameter of lock()/lockRead() was documented only as
"how long to wait to acquire lock until returning false", with no mention
of what the default value of 0 does. The README intro further implied the
call always blocks until the lock is released, which is misleading.

Per the RoadRunner lock plugin (roadrunner-server/lock, rpc.go): a wait of
0 is floored to defaultImmediateTimeout (1ms), so the call is effectively
non-blocking and returns false almost immediately when the resource is
already locked; a positive wait blocks up to that duration.

Clarify the docblocks and README accordingly so the behavior no longer has
to be reverse-engineered.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 974305af-2839-434f-b86d-e13b4e827f29

📥 Commits

Reviewing files that changed from the base of the PR and between 908f2ec and 37a7da9.

📒 Files selected for processing (3)
  • README.md
  • src/Lock.php
  • src/LockInterface.php
✅ Files skipped from review due to trivial changes (3)
  • src/LockInterface.php
  • README.md
  • src/Lock.php

📝 Walkthrough

Walkthrough

This PR updates LockInterface.php, Lock.php, and README.md to clarify that lock() and lockRead() are non-blocking by default when waitTTL is 0, use a server-capped acquire window of about 1ms, and return either a lock id or false when a positive wait expires. No code logic changed.

Changes

Lock wait semantics documentation

Layer / File(s) Summary
Interface contract documentation
src/LockInterface.php
Docblocks for lock() and lockRead() expanded to describe default non-blocking waitTTL behavior and the server acquire-window cap.
Implementation documentation
src/Lock.php
lock() and lockRead() PHPDoc updated to match the interface semantics for non-blocking default waits and timeout outcomes.
README usage documentation
README.md
Usage docs updated to describe non-blocking defaults, the 1ms window, waitTTL examples, and timeout behavior for lock() and lockRead().

Estimated code review effort: 1 (Trivial) | ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main documentation change about waitTTL defaults and the 1ms server cap.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gam6itko gam6itko marked this pull request as draft July 8, 2026 17:05

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@README.md`:
- Around line 50-55: The README still uses the old wait parameter name in the
lock documentation, which conflicts with the PHP API. Update the prose and both
examples for lock() and lockRead() to use waitTTL consistently instead of wait,
and make sure the surrounding text describes the timeout using the same symbol
so copied snippets match the actual method signatures.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 64ed6282-2c05-4ad6-9a6f-2d818d8e87f2

📥 Commits

Reviewing files that changed from the base of the PR and between 2c8e657 and 908f2ec.

📒 Files selected for processing (3)
  • README.md
  • src/Lock.php
  • src/LockInterface.php

Comment thread README.md Outdated
Consolidate resource/id/ttl/wait types, units (seconds or DateInterval),
defaults and the return value into a single scannable table, so the
per-argument behavior no longer has to be pieced together from prose and
the source.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@gam6itko gam6itko marked this pull request as ready for review July 8, 2026 17:10
The named-argument examples used `wait:`, but the parameter is `$waitTTL`,
so the snippets would fail with "Unknown named parameter $wait". Rename the
parameter references (named args, prose, table) to `waitTTL` so copied
snippets match lock()/lockRead(). Addresses CodeRabbit review feedback.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@roxblnfk roxblnfk merged commit da67151 into roadrunner-php:1.x Jul 9, 2026
10 of 12 checks passed
@roxblnfk roxblnfk changed the title Document waitTTL behavior (non-blocking default, 1ms server cap) Improve documentation Jul 9, 2026
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