After a Drydock lease triggers a resource to be reclaimed, stop it from triggering another reclaim until the first one completes

@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

fork

Summary:
Depends on D19752. Ref T13210. If resources take a long time to reclaim/destroy (normally, more than 15 seconds) a single new lease may update several times during the reclaim/destroy process and end up reclaiming multiple resources.

Instead: after a lease triggers a reclaim, prevent it from triggering another reclaim as long as the resource it is reclaiming hasn't finished its reclaim/destroy cycle. Basically, each lease only gets to destroy one resource at a time.

Test Plan:
- Added a `sleep(120)` to `destroyResource()` to simulate a long reclaim/destroy cycle.
- Allocated A, A, A working copies. Leased a B working copy.
- Before patch: saw "B" lease destroy all three "A" working copies after ~0, ~15, and ~30 seconds, then build a new "B" resource after ~120 seconds (when the first reclaim/destroy finished).
- After patch: saw "B" lease destroy one "A" working copy after ~0 seconds, then wait patiently until it finished up, then build a new "B" resource.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13210

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

epriestley 7 years ago 78ab675b e9309fdd

+32 -3

2 changed files

expand all

src

applications

drydock

worker

DrydockLeaseUpdateWorker.php

DrydockWorker.php

+20

src/applications/drydock/worker/DrydockLeaseUpdateWorker.php

··· 666 666 DrydockLease $lease) { 667 667 $viewer = $this->getViewer(); 668 668 669 + // If this lease is marked as already in the process of reclaiming a 670 + // resource, don't let it reclaim another one until the first reclaim 671 + // completes. This stops one lease from reclaiming a large number of 672 + // resources if the reclaims take a while to complete. 673 + $reclaiming_phid = $lease->getAttribute('drydock.reclaimingPHID'); 674 + if ($reclaiming_phid) { 675 + $reclaiming_resource = id(new DrydockResourceQuery()) 676 + ->setViewer($viewer) 677 + ->withPHIDs(array($reclaiming_phid)) 678 + ->withStatuses( 679 + array( 680 + DrydockResourceStatus::STATUS_ACTIVE, 681 + DrydockResourceStatus::STATUS_RELEASED, 682 + )) 683 + ->executeOne(); 684 + if ($reclaiming_resource) { 685 + return null; 686 + } 687 + } 688 + 669 689 $resources = id(new DrydockResourceQuery()) 670 690 ->setViewer($viewer) 671 691 ->withBlueprintPHIDs(array($blueprint->getPHID()))

+12 -3

src/applications/drydock/worker/DrydockWorker.php

··· 241 241 DrydockLease $lease) { 242 242 $viewer = $this->getViewer(); 243 243 244 + // Mark the lease as reclaiming this resource. It won't be allowed to start 245 + // another reclaim as long as this resource is still in the process of 246 + // being reclaimed. 247 + $lease->setAttribute('drydock.reclaimingPHID', $resource->getPHID()); 248 + 244 249 // When the resource releases, we we want to reawaken this task since it 245 - // should be able to start building a new resource right away. 250 + // should (usually) be able to start building a new resource right away. 246 251 $worker_task_id = $this->getCurrentWorkerTaskID(); 247 252 248 253 $command = DrydockCommand::initializeNewCommand($viewer) 249 254 ->setTargetPHID($resource->getPHID()) 250 255 ->setAuthorPHID($lease->getPHID()) 251 256 ->setCommand(DrydockCommand::COMMAND_RECLAIM) 252 - ->setProperty('awakenTaskIDs', array($worker_task_id)) 253 - ->save(); 257 + ->setProperty('awakenTaskIDs', array($worker_task_id)); 258 + 259 + $lease->openTransaction(); 260 + $lease->save(); 261 + $command->save(); 262 + $lease->saveTransaction(); 254 263 255 264 $resource->scheduleUpdate(); 256 265

Configure Feed

Configure Feed