@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
Merge the DrydockResource workers into a single worker
Summary:
Ref T9252. Currently, Drydock Leases and Resources have several workers:
- Resources: ResourceWorker, ResourceUpdateWorker, ResourceDestroyWorker
- Leases: AllocatorWorker, LeaseWorker, LeaseUpdateWorker, LeaseDestroyWorker
This is kind of a lot of stuff, and it creates some problems.
In particular, leases and resources in early lifecycle phases (pending/allocating/acquiring) can't process commands yet, because that code is only in the "UpdateWorker" classes. If they aren't able to move forward because of a bug, they also can't be released because they can't react to the release command until later in their lifecycle. This creates a soft hang where I have to go wipe stuff out of the database since there's no other way to get rid of it.
Instead, I want leases and resources to be releasable from any (pre-release / pre-destroy) phase of their lifecycle. To support this, all the workers before the "UpdateWorker" need to be able to process commands.
A second, similar issue is that logging and exception handling behaviors are underpowered right now. Elsewhere I began improving this, but ran into issues where all of the workers needed to share very similar exception code. Merging them will make this future change simpler.
This diff fixes this for resources: it merges the Worker, UpdateWorker and DestroyWorker logic into UpdateWorker and throws away the other two workers.
Test Plan: Nothing substantive yet, see next diff. I'll do the same thing for Leases, then test both more thoroughly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14201
+107
-109
-4
src/__phutil_library_map__.php
-4
src/__phutil_library_map__.php
···
867
867
'DrydockResource' => 'applications/drydock/storage/DrydockResource.php',
868
868
'DrydockResourceController' => 'applications/drydock/controller/DrydockResourceController.php',
869
869
'DrydockResourceDatasource' => 'applications/drydock/typeahead/DrydockResourceDatasource.php',
870
-
'DrydockResourceDestroyWorker' => 'applications/drydock/worker/DrydockResourceDestroyWorker.php',
871
870
'DrydockResourceListController' => 'applications/drydock/controller/DrydockResourceListController.php',
872
871
'DrydockResourceListView' => 'applications/drydock/view/DrydockResourceListView.php',
873
872
'DrydockResourcePHIDType' => 'applications/drydock/phid/DrydockResourcePHIDType.php',
···
877
876
'DrydockResourceStatus' => 'applications/drydock/constants/DrydockResourceStatus.php',
878
877
'DrydockResourceUpdateWorker' => 'applications/drydock/worker/DrydockResourceUpdateWorker.php',
879
878
'DrydockResourceViewController' => 'applications/drydock/controller/DrydockResourceViewController.php',
880
-
'DrydockResourceWorker' => 'applications/drydock/worker/DrydockResourceWorker.php',
881
879
'DrydockSFTPFilesystemInterface' => 'applications/drydock/interface/filesystem/DrydockSFTPFilesystemInterface.php',
882
880
'DrydockSSHCommandInterface' => 'applications/drydock/interface/command/DrydockSSHCommandInterface.php',
883
881
'DrydockSlotLock' => 'applications/drydock/storage/DrydockSlotLock.php',
···
4618
4616
),
4619
4617
'DrydockResourceController' => 'DrydockController',
4620
4618
'DrydockResourceDatasource' => 'PhabricatorTypeaheadDatasource',
4621
-
'DrydockResourceDestroyWorker' => 'DrydockWorker',
4622
4619
'DrydockResourceListController' => 'DrydockResourceController',
4623
4620
'DrydockResourceListView' => 'AphrontView',
4624
4621
'DrydockResourcePHIDType' => 'PhabricatorPHIDType',
···
4628
4625
'DrydockResourceStatus' => 'DrydockConstants',
4629
4626
'DrydockResourceUpdateWorker' => 'DrydockWorker',
4630
4627
'DrydockResourceViewController' => 'DrydockResourceController',
4631
-
'DrydockResourceWorker' => 'DrydockWorker',
4632
4628
'DrydockSFTPFilesystemInterface' => 'DrydockFilesystemInterface',
4633
4629
'DrydockSSHCommandInterface' => 'DrydockCommandInterface',
4634
4630
'DrydockSlotLock' => 'DrydockDAO',
+6
-4
src/applications/drydock/storage/DrydockResource.php
+6
-4
src/applications/drydock/storage/DrydockResource.php
···
246
246
}
247
247
}
248
248
249
-
public function canUpdate() {
249
+
public function canReceiveCommands() {
250
250
switch ($this->getStatus()) {
251
-
case DrydockResourceStatus::STATUS_ACTIVE:
252
-
return true;
253
-
default:
251
+
case DrydockResourceStatus::STATUS_RELEASED:
252
+
case DrydockResourceStatus::STATUS_BROKEN:
253
+
case DrydockResourceStatus::STATUS_DESTROYED:
254
254
return false;
255
+
default:
256
+
return true;
255
257
}
256
258
}
257
259
+1
-1
src/applications/drydock/worker/DrydockAllocatorWorker.php
+1
-1
src/applications/drydock/worker/DrydockAllocatorWorker.php
-35
src/applications/drydock/worker/DrydockResourceDestroyWorker.php
-35
src/applications/drydock/worker/DrydockResourceDestroyWorker.php
···
1
-
<?php
2
-
3
-
final class DrydockResourceDestroyWorker extends DrydockWorker {
4
-
5
-
protected function doWork() {
6
-
$resource_phid = $this->getTaskDataValue('resourcePHID');
7
-
$resource = $this->loadResource($resource_phid);
8
-
$this->destroyResource($resource);
9
-
}
10
-
11
-
private function destroyResource(DrydockResource $resource) {
12
-
$status = $resource->getStatus();
13
-
14
-
switch ($status) {
15
-
case DrydockResourceStatus::STATUS_RELEASED:
16
-
case DrydockResourceStatus::STATUS_BROKEN:
17
-
break;
18
-
default:
19
-
throw new PhabricatorWorkerPermanentFailureException(
20
-
pht(
21
-
'Unable to destroy resource ("%s"), resource has the wrong '.
22
-
'status ("%s").',
23
-
$resource->getPHID(),
24
-
$status));
25
-
}
26
-
27
-
$blueprint = $resource->getBlueprint();
28
-
$blueprint->destroyResource($resource);
29
-
30
-
$resource
31
-
->setStatus(DrydockResourceStatus::STATUS_DESTROYED)
32
-
->save();
33
-
}
34
-
35
-
}
+99
-19
src/applications/drydock/worker/DrydockResourceUpdateWorker.php
+99
-19
src/applications/drydock/worker/DrydockResourceUpdateWorker.php
···
1
1
<?php
2
2
3
+
/**
4
+
* @task command Processing Commands
5
+
* @task activate Activating Resources
6
+
* @task release Releasing Resources
7
+
* @task destroy Destroying Resources
8
+
*/
3
9
final class DrydockResourceUpdateWorker extends DrydockWorker {
4
10
5
11
protected function doWork() {
···
23
29
}
24
30
25
31
private function updateResource(DrydockResource $resource) {
26
-
if (!$resource->canUpdate()) {
32
+
$this->processResourceCommands($resource);
33
+
34
+
$resource_status = $resource->getStatus();
35
+
switch ($resource_status) {
36
+
case DrydockResourceStatus::STATUS_PENDING:
37
+
$this->activateResource($resource);
38
+
break;
39
+
case DrydockResourceStatus::STATUS_ACTIVE:
40
+
// Nothing to do.
41
+
break;
42
+
case DrydockResourceStatus::STATUS_RELEASED:
43
+
case DrydockResourceStatus::STATUS_BROKEN:
44
+
$this->destroyResource($resource);
45
+
break;
46
+
case DrydockResourceStatus::STATUS_DESTROYED:
47
+
// Nothing to do.
48
+
break;
49
+
}
50
+
51
+
$this->yieldIfExpiringResource($resource);
52
+
}
53
+
54
+
55
+
/* -( Processing Commands )------------------------------------------------ */
56
+
57
+
58
+
/**
59
+
* @task command
60
+
*/
61
+
private function processResourceCommands(DrydockResource $resource) {
62
+
if (!$resource->canReceiveCommands()) {
27
63
return;
28
64
}
29
65
···
31
67
32
68
$commands = $this->loadCommands($resource->getPHID());
33
69
foreach ($commands as $command) {
34
-
if (!$resource->canUpdate()) {
70
+
if (!$resource->canReceiveCommands()) {
35
71
break;
36
72
}
37
73
38
-
$this->processCommand($resource, $command);
74
+
$this->processResourceCommand($resource, $command);
39
75
40
76
$command
41
77
->setIsConsumed(true)
42
78
->save();
43
79
}
80
+
}
44
81
45
-
$this->yieldIfExpiringResource($resource);
46
-
}
47
82
48
-
private function processCommand(
83
+
/**
84
+
* @task command
85
+
*/
86
+
private function processResourceCommand(
49
87
DrydockResource $resource,
50
88
DrydockCommand $command) {
51
89
···
56
94
}
57
95
}
58
96
59
-
private function releaseResource(DrydockResource $resource) {
60
-
if ($resource->getStatus() != DrydockResourceStatus::STATUS_ACTIVE) {
61
-
// If we had multiple release commands
62
-
// This command is only meaningful to resources in the "Open" state.
63
-
return;
97
+
98
+
/* -( Activating Resources )----------------------------------------------- */
99
+
100
+
101
+
/**
102
+
* @task activate
103
+
*/
104
+
private function activateResource(DrydockResource $resource) {
105
+
$blueprint = $resource->getBlueprint();
106
+
$blueprint->activateResource($resource);
107
+
$this->validateActivatedResource($blueprint, $resource);
108
+
}
109
+
110
+
111
+
/**
112
+
* @task activate
113
+
*/
114
+
private function validateActivatedResource(
115
+
DrydockBlueprint $blueprint,
116
+
DrydockResource $resource) {
117
+
118
+
if (!$resource->isActivatedResource()) {
119
+
throw new Exception(
120
+
pht(
121
+
'Blueprint "%s" (of type "%s") is not properly implemented: %s '.
122
+
'must actually allocate the resource it returns.',
123
+
$blueprint->getBlueprintName(),
124
+
$blueprint->getClassName(),
125
+
'allocateResource()'));
64
126
}
65
127
128
+
}
129
+
130
+
131
+
/* -( Releasing Resources )------------------------------------------------ */
132
+
133
+
134
+
/**
135
+
* @task release
136
+
*/
137
+
private function releaseResource(DrydockResource $resource) {
66
138
$viewer = $this->getViewer();
67
139
$drydock_phid = id(new PhabricatorDrydockApplication())->getPHID();
68
140
···
97
169
$lease->scheduleUpdate();
98
170
}
99
171
100
-
PhabricatorWorker::scheduleTask(
101
-
'DrydockResourceDestroyWorker',
102
-
array(
103
-
'resourcePHID' => $resource->getPHID(),
104
-
),
105
-
array(
106
-
'objectPHID' => $resource->getPHID(),
107
-
));
172
+
$this->destroyResource($resource);
108
173
}
109
174
175
+
176
+
/* -( Destroying Resources )----------------------------------------------- */
177
+
178
+
179
+
/**
180
+
* @task destroy
181
+
*/
182
+
private function destroyResource(DrydockResource $resource) {
183
+
$blueprint = $resource->getBlueprint();
184
+
$blueprint->destroyResource($resource);
185
+
186
+
$resource
187
+
->setStatus(DrydockResourceStatus::STATUS_DESTROYED)
188
+
->save();
189
+
}
110
190
}
-45
src/applications/drydock/worker/DrydockResourceWorker.php
-45
src/applications/drydock/worker/DrydockResourceWorker.php
···
1
-
<?php
2
-
3
-
final class DrydockResourceWorker extends DrydockWorker {
4
-
5
-
protected function doWork() {
6
-
$resource_phid = $this->getTaskDataValue('resourcePHID');
7
-
$resource = $this->loadResource($resource_phid);
8
-
9
-
$this->activateResource($resource);
10
-
}
11
-
12
-
13
-
private function activateResource(DrydockResource $resource) {
14
-
$resource_status = $resource->getStatus();
15
-
16
-
if ($resource_status != DrydockResourceStatus::STATUS_PENDING) {
17
-
throw new PhabricatorWorkerPermanentFailureException(
18
-
pht(
19
-
'Trying to activate resource from wrong status ("%s").',
20
-
$resource_status));
21
-
}
22
-
23
-
$blueprint = $resource->getBlueprint();
24
-
$blueprint->activateResource($resource);
25
-
$this->validateActivatedResource($blueprint, $resource);
26
-
}
27
-
28
-
29
-
private function validateActivatedResource(
30
-
DrydockBlueprint $blueprint,
31
-
DrydockResource $resource) {
32
-
33
-
if (!$resource->isActivatedResource()) {
34
-
throw new Exception(
35
-
pht(
36
-
'Blueprint "%s" (of type "%s") is not properly implemented: %s '.
37
-
'must actually allocate the resource it returns.',
38
-
$blueprint->getBlueprintName(),
39
-
$blueprint->getClassName(),
40
-
'allocateResource()'));
41
-
}
42
-
43
-
}
44
-
45
-
}