-
Notifications
You must be signed in to change notification settings - Fork 11.8k
[12.x] Ability to refresh cache locks #58349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 12.x
Are you sure you want to change the base?
Changes from all commits
e33a9e0
adbe10e
2ae7655
b188cac
b2c53e3
175576e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,4 +102,23 @@ public function forceRelease() | |
| { | ||
| unset($this->store->locks[$this->name]); | ||
| } | ||
|
|
||
| /** | ||
| * Attempt to refresh the lock for the given number of seconds. | ||
| * | ||
| * @param int|null $seconds | ||
| * @return bool | ||
| */ | ||
| public function refresh($seconds = null) | ||
| { | ||
| if (! $this->isOwnedByCurrentProcess()) { | ||
| return false; | ||
| } | ||
|
|
||
| $seconds ??= $this->seconds; | ||
|
|
||
| $this->store->locks[$this->name]['expiresAt'] = $seconds === 0 ? null : Carbon::now()->addSeconds($seconds); | ||
|
|
||
| return true; | ||
| } | ||
|
Comment on lines
+112
to
+123
|
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -170,6 +170,24 @@ protected function getCurrentOwner() | |||||||||||||||||||||||||
| return $this->connection->table($this->table)->where('key', $this->name)->first()?->owner; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * Attempt to refresh the lock for the given number of seconds. | ||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||
| * @param int|null $seconds | ||||||||||||||||||||||||||
| * @return bool | ||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| public function refresh($seconds = null) | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| $seconds ??= $this->seconds; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| $this->seconds = $seconds; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return $this->connection->table($this->table) | ||||||||||||||||||||||||||
| ->where('key', $this->name) | ||||||||||||||||||||||||||
| ->where('owner', $this->owner) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| ->where('owner', $this->owner) | |
| ->where('owner', $this->owner) | |
| ->where('expiration', '>', $this->currentTime()) |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The refresh operation modifies the instance's $seconds property as a side effect. This state mutation is unexpected for a method that's meant to atomically refresh a lock's expiration. If refresh is called multiple times with different durations, the instance state will change each time. Consider removing this line to avoid side effects, as the refresh operation should only update the lock's expiration in the storage backend without modifying the lock instance's state.
| $this->seconds = $seconds; | |
| return $this->connection->table($this->table) | |
| ->where('key', $this->name) | |
| ->where('owner', $this->owner) | |
| ->update(['expiration' => $this->expiresAt()]) >= 1; | |
| $expiration = $this->currentTime() + $seconds; | |
| return $this->connection->table($this->table) | |
| ->where('key', $this->name) | |
| ->where('owner', $this->owner) | |
| ->update(['expiration' => $expiration]) >= 1; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -73,4 +73,21 @@ protected function getCurrentOwner() | |
| { | ||
| return $this->dynamo->get($this->name); | ||
| } | ||
|
|
||
| /** | ||
| * Attempt to refresh the lock for the given number of seconds. | ||
| * | ||
| * @param int|null $seconds | ||
| * @return bool | ||
| */ | ||
| public function refresh($seconds = null) | ||
| { | ||
| $seconds ??= $this->seconds; | ||
|
|
||
| if ($seconds <= 0) { | ||
| $seconds = 86400; | ||
| } | ||
bytestream marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| return $this->dynamo->refreshIfOwned($this->name, $this->owner, $seconds); | ||
| } | ||
|
Comment on lines
+83
to
+92
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,4 +71,23 @@ protected function getCurrentOwner() | |
| { | ||
| return $this->memcached->get($this->name); | ||
| } | ||
|
|
||
| /** | ||
| * Attempt to refresh the lock for the given number of seconds. | ||
| * | ||
| * @param int|null $seconds | ||
| * @return bool | ||
| */ | ||
| public function refresh($seconds = null) | ||
| { | ||
| $seconds ??= $this->seconds; | ||
|
|
||
| $value = $this->memcached->get($this->name, null, \Memcached::GET_EXTENDED); | ||
|
|
||
| if ($value === false || ($value['value'] ?? null) !== $this->owner) { | ||
| return false; | ||
| } | ||
|
|
||
| return $this->memcached->cas($value['cas'], $this->name, $this->owner, $seconds); | ||
| } | ||
|
Comment on lines
+81
to
+92
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,4 +43,15 @@ protected function getCurrentOwner() | |
| { | ||
| return $this->owner; | ||
| } | ||
|
|
||
| /** | ||
| * Attempt to refresh the lock for the given number of seconds. | ||
| * | ||
| * @param int|null $seconds | ||
| * @return bool | ||
| */ | ||
| public function refresh($seconds = null) | ||
| { | ||
| return true; | ||
| } | ||
|
Comment on lines
+53
to
+56
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ArrayLock refresh functionality lacks test coverage. While CacheArrayStoreTest has comprehensive tests for lock acquisition and release, there are no tests for the newly added refresh method. Consider adding test cases that verify: (1) successful refresh, (2) refresh preventing acquisition by another process, (3) refresh failure when attempted by another owner, (4) refresh with default seconds parameter, and (5) refresh behavior with expired locks.