recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Reduce collision rate for concurrency-limiting slot locks

Summary:
Depends on D19077. Ref T13073. When we're using slot locks to enforce a limit (e.g., maximum of 5 simultaneous things) we currently load locks owned by the blueprint to identify which slots are likely to be free.

However, this isn't right: the blueprint doesn't own these locks. The resources do.

We still get the right behavior eventually, but we incorrectly identify that every slot lock is always free, so as the slots fill up we'll tend to guess wrong more and more often.

Instead, load the slot locks by name explicitly.

Test Plan: Implemented lock-based limiting on `HoaxBlueprint`, `var_dump()`'d the candidate locks, saw correct test state for locks. Acquired leases without releasing, got all of the slots filled without any slot lock collisions (previously, the last slot or two tended to collide a lot).

Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13073

Differential Revision: https://secure.phabricator.com/D19078

epriestley 619943be 2994753d

+12 -7
+12 -7
src/applications/drydock/blueprint/DrydockBlueprintImplementation.php
··· 396 396 } 397 397 398 398 // For reasonable limits, actually check for an available slot. 399 - $locks = DrydockSlotLock::loadLocks($blueprint_phid); 400 - $locks = mpull($locks, null, 'getLockKey'); 401 - 402 399 $slots = range(0, $limit - 1); 403 400 shuffle($slots); 404 401 402 + $lock_names = array(); 405 403 foreach ($slots as $slot) { 406 - $slot_lock = "allocator({$blueprint_phid}).limit({$slot})"; 407 - if (empty($locks[$slot_lock])) { 408 - return $slot_lock; 404 + $lock_names[] = "allocator({$blueprint_phid}).limit({$slot})"; 405 + } 406 + 407 + $locks = DrydockSlotLock::loadHeldLocks($lock_names); 408 + $locks = mpull($locks, null, 'getLockKey'); 409 + 410 + foreach ($lock_names as $lock_name) { 411 + if (empty($locks[$lock_name])) { 412 + return $lock_name; 409 413 } 410 414 } 411 415 ··· 414 418 // lock will be free by the time we try to take it, but usually we'll just 415 419 // fail to grab the lock, throw an appropriate lock exception, and get back 416 420 // on the right path to retry later. 417 - return $slot_lock; 421 + 422 + return $lock_name; 418 423 } 419 424 420 425