@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
Improve error and exception handling for Drydock leases
Summary:
Ref T9252. See companion change in D14211. This does the same thing for leases.
Particularly, most of the TODOs about error handling can just be removed because they'll do the right things by default now.
This and D14211 also move slot lock release to after resource destruction. This feels cleaner than trying to release early at release/break.
Test Plan: Restarted a Harbormaster build, got a clean build result. This needs more vetting but I'll clean up any issues as I hit them.
Reviewers: chad, hach-que
Reviewed By: hach-que
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14212
+88
-29
+88
-29
src/applications/drydock/worker/DrydockLeaseUpdateWorker.php
+88
-29
src/applications/drydock/worker/DrydockLeaseUpdateWorker.php
···
7
7
* @task acquire Acquiring Leases
8
8
* @task activate Activating Leases
9
9
* @task release Releasing Leases
10
+
* @task break Breaking Leases
10
11
* @task destroy Destroying Leases
11
12
*/
12
13
final class DrydockLeaseUpdateWorker extends DrydockWorker {
···
22
23
23
24
try {
24
25
$lease = $this->loadLease($lease_phid);
25
-
$this->updateLease($lease);
26
+
$this->handleUpdate($lease);
26
27
} catch (Exception $ex) {
27
28
$lock->unlock();
28
29
throw $ex;
···
38
39
/**
39
40
* @task update
40
41
*/
42
+
private function handleUpdate(DrydockLease $lease) {
43
+
try {
44
+
$this->updateLease($lease);
45
+
} catch (Exception $ex) {
46
+
if ($this->isTemporaryException($ex)) {
47
+
$this->yieldLease($lease, $ex);
48
+
} else {
49
+
$this->breakLease($lease, $ex);
50
+
}
51
+
}
52
+
}
53
+
54
+
55
+
/**
56
+
* @task update
57
+
*/
41
58
private function updateLease(DrydockLease $lease) {
42
59
$this->processLeaseCommands($lease);
43
60
···
64
81
}
65
82
66
83
84
+
/**
85
+
* @task update
86
+
*/
87
+
private function yieldLease(DrydockLease $lease, Exception $ex) {
88
+
$duration = $this->getYieldDurationFromException($ex);
89
+
90
+
$lease->logEvent(
91
+
DrydockLeaseActivationYieldLogType::LOGCONST,
92
+
array(
93
+
'duration' => $duration,
94
+
));
95
+
96
+
throw new PhabricatorWorkerYieldException($duration);
97
+
}
98
+
99
+
67
100
/* -( Processing Commands )------------------------------------------------ */
68
101
69
102
···
145
178
// satisfy the lease, just not right now. This is a temporary failure,
146
179
// and we expect allocation to succeed eventually.
147
180
if (!$usable_blueprints) {
148
-
// TODO: More formal temporary failure here. We should retry this
149
-
// "soon" but not "immediately".
150
-
throw new Exception(
151
-
pht('No blueprints have space to allocate a resource right now.'));
181
+
$lease->logEvent(
182
+
DrydockLeaseWaitingForResourcesLogType::LOGCONST,
183
+
array(
184
+
'blueprintPHIDs' => mpull($blueprints, 'getPHID'),
185
+
));
186
+
187
+
throw new PhabricatorWorkerYieldException(15);
152
188
}
153
189
154
190
$usable_blueprints = $this->rankBlueprints($usable_blueprints, $lease);
···
167
203
}
168
204
169
205
if (!$resources) {
170
-
// TODO: We should distinguish between temporary and permament failures
171
-
// here. If any blueprint failed temporarily, retry "soon". If none
172
-
// of these failures were temporary, maybe this should be a permanent
173
-
// failure?
174
206
throw new PhutilAggregateException(
175
207
pht(
176
208
'All blueprints failed to allocate a suitable new resource when '.
···
200
232
}
201
233
202
234
if (!$allocated) {
203
-
// TODO: We should distinguish between temporary and permanent failures
204
-
// here. If any failures were temporary (specifically, failed to acquire
205
-
// locks)
206
-
207
235
throw new PhutilAggregateException(
208
236
pht(
209
237
'Unable to acquire lease "%s" on any resouce.',
···
471
499
$lease_type = $lease->getResourceType();
472
500
473
501
if ($resource_type !== $lease_type) {
474
-
// TODO: Destroy the resource here?
475
-
476
502
throw new Exception(
477
503
pht(
478
504
'Blueprint "%s" (of type "%s") is not properly implemented: it '.
···
549
575
$resource_phid = $resource->getPHID();
550
576
551
577
if ($lease_phid !== $resource_phid) {
552
-
// TODO: Destroy the lease?
553
578
throw new Exception(
554
579
pht(
555
580
'Blueprint "%s" (of type "%s") is not properly implemented: it '.
···
570
595
private function activateLease(DrydockLease $lease) {
571
596
$resource = $lease->getResource();
572
597
if (!$resource) {
573
-
throw new PhabricatorWorkerPermanentFailureException(
598
+
throw new Exception(
574
599
pht('Trying to activate lease with no resource.'));
575
600
}
576
601
577
602
$resource_status = $resource->getStatus();
578
603
579
604
if ($resource_status == DrydockResourceStatus::STATUS_PENDING) {
580
-
// TODO: This is explicitly a temporary failure -- we are waiting for
581
-
// the resource to come up.
582
-
throw new Exception(pht('Resource still activating.'));
605
+
throw new PhabricatorWorkerYieldException(15);
583
606
}
584
607
585
608
if ($resource_status != DrydockResourceStatus::STATUS_ACTIVE) {
586
-
throw new PhabricatorWorkerPermanentFailureException(
609
+
throw new Exception(
587
610
pht(
588
611
'Trying to activate lease on a dead resource (in status "%s").',
589
612
$resource_status));
···
630
653
* @task release
631
654
*/
632
655
private function releaseLease(DrydockLease $lease) {
633
-
$lease->openTransaction();
634
-
$lease
635
-
->setStatus(DrydockLeaseStatus::STATUS_RELEASED)
636
-
->save();
637
-
638
-
// TODO: Hold slot locks until destruction?
639
-
DrydockSlotLock::releaseLocks($lease->getPHID());
640
-
$lease->saveTransaction();
656
+
$lease
657
+
->setStatus(DrydockLeaseStatus::STATUS_RELEASED)
658
+
->save();
641
659
642
660
$lease->logEvent(DrydockLeaseReleasedLogType::LOGCONST);
643
661
···
650
668
}
651
669
652
670
671
+
/* -( Breaking Leases )---------------------------------------------------- */
672
+
673
+
674
+
/**
675
+
* @task break
676
+
*/
677
+
protected function breakLease(DrydockLease $lease, Exception $ex) {
678
+
switch ($lease->getStatus()) {
679
+
case DrydockLeaseStatus::STATUS_BROKEN:
680
+
case DrydockLeaseStatus::STATUS_RELEASED:
681
+
case DrydockLeaseStatus::STATUS_DESTROYED:
682
+
throw new PhutilProxyException(
683
+
pht(
684
+
'Unexpected failure while destroying lease ("%s").',
685
+
$lease->getPHID()),
686
+
$ex);
687
+
}
688
+
689
+
$lease
690
+
->setStatus(DrydockLeaseStatus::STATUS_BROKEN)
691
+
->save();
692
+
693
+
$lease->scheduleDestruction();
694
+
695
+
$lease->logEvent(
696
+
DrydockLeaseActivationFailureLogType::LOGCONST,
697
+
array(
698
+
'class' => get_class($ex),
699
+
'message' => $ex->getMessage(),
700
+
));
701
+
702
+
throw new PhabricatorWorkerPermanentFailureException(
703
+
pht(
704
+
'Permanent failure while activating lease ("%s"): %s',
705
+
$lease->getPHID(),
706
+
$ex->getMessage()));
707
+
}
708
+
709
+
653
710
/* -( Destroying Leases )-------------------------------------------------- */
654
711
655
712
···
661
718
$blueprint = $resource->getBlueprint();
662
719
663
720
$blueprint->destroyLease($resource, $lease);
721
+
722
+
DrydockSlotLock::releaseLocks($lease->getPHID());
664
723
665
724
$lease
666
725
->setStatus(DrydockLeaseStatus::STATUS_DESTROYED)