2012-11-07 00:28:33 +01:00
|
|
|
<?php
|
|
|
|
|
|
2013-12-26 21:30:36 +01:00
|
|
|
final class DrydockLeaseViewController extends DrydockLeaseController {
|
2012-11-07 00:28:33 +01:00
|
|
|
|
2015-07-27 16:56:52 +02:00
|
|
|
public function handleRequest(AphrontRequest $request) {
|
|
|
|
|
$viewer = $request->getViewer();
|
|
|
|
|
$id = $request->getURIData('id');
|
2012-11-07 00:28:33 +01:00
|
|
|
|
2013-12-27 22:15:19 +01:00
|
|
|
$lease = id(new DrydockLeaseQuery())
|
|
|
|
|
->setViewer($viewer)
|
2015-07-27 16:56:52 +02:00
|
|
|
->withIDs(array($id))
|
2015-09-28 18:35:26 +02:00
|
|
|
->needUnconsumedCommands(true)
|
2013-12-27 22:15:19 +01:00
|
|
|
->executeOne();
|
2012-11-07 00:28:33 +01:00
|
|
|
if (!$lease) {
|
|
|
|
|
return new Aphront404Response();
|
|
|
|
|
}
|
|
|
|
|
|
Move Drydock logs to PHIDs and increased structure
Summary:
Ref T9252. Several general changes here:
- Moves logs to use PHIDs instead of IDs. This generally improves flexibility (for example, it's a lot easier to render handles).
- Adds `blueprintPHID` to logs. Although you can usually figure this out from the leasePHID or resourcePHID, it lets us query relevant logs on Blueprint views.
- Instead of making logs a top-level object, make them strictly a sub-object of Blueprints, Resources and Leases. So you go Drydock > Lease > Logs, etc., to get to logs.
- I might restore the "everything" view eventually, but it doesn't interact well with policies and I'm not sure it's very useful. A policy-violating `bin/drydock log` might be cleaner.
- Policy-wise, we always show you that logs exist, we just don't show you log content if it's about something you can't see. This is similar to seeing restricted handles in other applications.
- Instead of just having a message, give logs "type" + "data". This will let logs be more structured and translatable. This is similar to recent changes to Herald which seem to have worked well.
Test Plan:
Added some placeholder log writes, viewed those logs in the UI.
{F855199}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14196
2015-10-01 17:06:23 +02:00
|
|
|
$id = $lease->getID();
|
|
|
|
|
$lease_uri = $this->getApplicationURI("lease/{$id}/");
|
2012-12-17 23:47:21 +01:00
|
|
|
|
|
|
|
|
$title = pht('Lease %d', $lease->getID());
|
2012-11-07 00:28:33 +01:00
|
|
|
|
2013-09-17 18:12:37 +02:00
|
|
|
$header = id(new PHUIHeaderView())
|
2012-11-07 00:28:33 +01:00
|
|
|
->setHeader($title);
|
|
|
|
|
|
2015-09-28 18:35:26 +02:00
|
|
|
if ($lease->isReleasing()) {
|
|
|
|
|
$header->setStatus('fa-exclamation-triangle', 'red', pht('Releasing'));
|
|
|
|
|
}
|
|
|
|
|
|
2012-11-07 00:28:33 +01:00
|
|
|
$actions = $this->buildActionListView($lease);
|
2013-10-11 16:53:56 +02:00
|
|
|
$properties = $this->buildPropertyListView($lease, $actions);
|
2012-11-07 00:28:33 +01:00
|
|
|
|
Move Drydock logs to PHIDs and increased structure
Summary:
Ref T9252. Several general changes here:
- Moves logs to use PHIDs instead of IDs. This generally improves flexibility (for example, it's a lot easier to render handles).
- Adds `blueprintPHID` to logs. Although you can usually figure this out from the leasePHID or resourcePHID, it lets us query relevant logs on Blueprint views.
- Instead of making logs a top-level object, make them strictly a sub-object of Blueprints, Resources and Leases. So you go Drydock > Lease > Logs, etc., to get to logs.
- I might restore the "everything" view eventually, but it doesn't interact well with policies and I'm not sure it's very useful. A policy-violating `bin/drydock log` might be cleaner.
- Policy-wise, we always show you that logs exist, we just don't show you log content if it's about something you can't see. This is similar to seeing restricted handles in other applications.
- Instead of just having a message, give logs "type" + "data". This will let logs be more structured and translatable. This is similar to recent changes to Herald which seem to have worked well.
Test Plan:
Added some placeholder log writes, viewed those logs in the UI.
{F855199}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14196
2015-10-01 17:06:23 +02:00
|
|
|
$log_query = id(new DrydockLogQuery())
|
|
|
|
|
->withLeasePHIDs(array($lease->getPHID()));
|
2012-11-07 00:28:33 +01:00
|
|
|
|
Move Drydock logs to PHIDs and increased structure
Summary:
Ref T9252. Several general changes here:
- Moves logs to use PHIDs instead of IDs. This generally improves flexibility (for example, it's a lot easier to render handles).
- Adds `blueprintPHID` to logs. Although you can usually figure this out from the leasePHID or resourcePHID, it lets us query relevant logs on Blueprint views.
- Instead of making logs a top-level object, make them strictly a sub-object of Blueprints, Resources and Leases. So you go Drydock > Lease > Logs, etc., to get to logs.
- I might restore the "everything" view eventually, but it doesn't interact well with policies and I'm not sure it's very useful. A policy-violating `bin/drydock log` might be cleaner.
- Policy-wise, we always show you that logs exist, we just don't show you log content if it's about something you can't see. This is similar to seeing restricted handles in other applications.
- Instead of just having a message, give logs "type" + "data". This will let logs be more structured and translatable. This is similar to recent changes to Herald which seem to have worked well.
Test Plan:
Added some placeholder log writes, viewed those logs in the UI.
{F855199}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14196
2015-10-01 17:06:23 +02:00
|
|
|
$log_box = $this->buildLogBox(
|
|
|
|
|
$log_query,
|
|
|
|
|
$this->getApplicationURI("lease/{$id}/logs/query/all/"));
|
2012-11-07 00:28:33 +01:00
|
|
|
|
2012-12-17 23:47:21 +01:00
|
|
|
$crumbs = $this->buildApplicationCrumbs();
|
2013-12-19 02:47:34 +01:00
|
|
|
$crumbs->addTextCrumb($title, $lease_uri);
|
2012-12-17 23:47:21 +01:00
|
|
|
|
2015-09-21 13:45:43 +02:00
|
|
|
$locks = $this->buildLocksTab($lease->getPHID());
|
Add a command queue to Drydock to manage lease/resource release
Summary:
Ref T9252. Broadly, Drydock currently races on releasing objects from the "active" state. To reproduce this:
- Scatter some sleep()s pretty much anywhere in the release code.
- Release several times from web UI or CLI in quick succession.
Resources or leases will execute some release code twice or otherwise do inconsistent things.
(I didn't chase down a detailed reproduction scenario for this since inspection of the code makes it clear that there are no meaningful locks or mechanisms preventing this.)
Instead, add a Harbormaster-style command queue to resources and leases. When something wants to do a release, it adds a command to the queue and schedules a worker. The workers acquire a lock, then try to consume commands from the queue.
This guarantees that only one process is responsible for writes to active resource/leases.
This is the last major step to giving resources and leases a single writer during all states:
- Resource, Unsaved: AllocatorWorker
- Resource, Pending: ResourceWorker (Possible rename to "Allocated?")
- Resource, Open: This diff, ResourceUpdateWorker. (Likely rename to "Active").
- Resource, Closed/Broken: Future destruction worker. (Likely rename to "Released" / "Broken"; maybe remove "Broken").
- Resource, Destroyed: No writes.
- Lease, Unsaved: Whatever wants the lease.
- Lease, Pending: AllocatorWorker
- Lease, Acquired: LeaseWorker
- Lease, Active: This diff, LeaseUpdateWorker.
- Lease, Released/Broken: Future destruction worker (Maybe remove "Broken"?)
- Lease, Expired: No writes. (Likely rename to "Destroyed").
In most phases, we can already guarantee that there is a single writer without doing any extra work. This is more complicated in the "Active" case because the release buttons on the web UI, the release tools on the CLI, the lease requestor itself, the garbage collector, and any other release process cleaning up related objects may try to effect a release. All of these could race one another (and, in many cases, race other processes from other phases because all of these get to act immediately) as this code is currently written. Using a queue here lets us make sure there's only a single writer in this phase.
One thing which is notable is that whatever acquires a lease **can not write to it**! It is never the writer once it queues the lease for activation. It can not write to any resources, either. And, likewise, Blueprints can not write to resources while acquiring or releasing leases.
We may need to provide a mechinism so that blueprints and/or resource/lease holders get to attach some storage to resources/leases for bookkeeping. For example, a blueprint might need to keep some kind of cache on a resource to help it manage state. But I think we can cross that bridge when we come to it, and nothing else would need to write to this storage so it's technically straightforward to introduce such a mechanism if we need one.
Test Plan:
- Viewed buttons in web UI, checked enabled/disabled states.
- Clicked the buttons.
- Saw commands show up in the command queue.
- Saw some daemon stuff get scheduled.
- Ran CLI tools, saw commands get consumed and resources/leases release.
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14143
2015-09-23 16:42:08 +02:00
|
|
|
$commands = $this->buildCommandsTab($lease->getPHID());
|
2015-09-21 13:45:43 +02:00
|
|
|
|
2013-09-29 00:55:38 +02:00
|
|
|
$object_box = id(new PHUIObjectBoxView())
|
|
|
|
|
->setHeader($header)
|
2015-09-21 13:45:43 +02:00
|
|
|
->addPropertyList($properties, pht('Properties'))
|
Add a command queue to Drydock to manage lease/resource release
Summary:
Ref T9252. Broadly, Drydock currently races on releasing objects from the "active" state. To reproduce this:
- Scatter some sleep()s pretty much anywhere in the release code.
- Release several times from web UI or CLI in quick succession.
Resources or leases will execute some release code twice or otherwise do inconsistent things.
(I didn't chase down a detailed reproduction scenario for this since inspection of the code makes it clear that there are no meaningful locks or mechanisms preventing this.)
Instead, add a Harbormaster-style command queue to resources and leases. When something wants to do a release, it adds a command to the queue and schedules a worker. The workers acquire a lock, then try to consume commands from the queue.
This guarantees that only one process is responsible for writes to active resource/leases.
This is the last major step to giving resources and leases a single writer during all states:
- Resource, Unsaved: AllocatorWorker
- Resource, Pending: ResourceWorker (Possible rename to "Allocated?")
- Resource, Open: This diff, ResourceUpdateWorker. (Likely rename to "Active").
- Resource, Closed/Broken: Future destruction worker. (Likely rename to "Released" / "Broken"; maybe remove "Broken").
- Resource, Destroyed: No writes.
- Lease, Unsaved: Whatever wants the lease.
- Lease, Pending: AllocatorWorker
- Lease, Acquired: LeaseWorker
- Lease, Active: This diff, LeaseUpdateWorker.
- Lease, Released/Broken: Future destruction worker (Maybe remove "Broken"?)
- Lease, Expired: No writes. (Likely rename to "Destroyed").
In most phases, we can already guarantee that there is a single writer without doing any extra work. This is more complicated in the "Active" case because the release buttons on the web UI, the release tools on the CLI, the lease requestor itself, the garbage collector, and any other release process cleaning up related objects may try to effect a release. All of these could race one another (and, in many cases, race other processes from other phases because all of these get to act immediately) as this code is currently written. Using a queue here lets us make sure there's only a single writer in this phase.
One thing which is notable is that whatever acquires a lease **can not write to it**! It is never the writer once it queues the lease for activation. It can not write to any resources, either. And, likewise, Blueprints can not write to resources while acquiring or releasing leases.
We may need to provide a mechinism so that blueprints and/or resource/lease holders get to attach some storage to resources/leases for bookkeeping. For example, a blueprint might need to keep some kind of cache on a resource to help it manage state. But I think we can cross that bridge when we come to it, and nothing else would need to write to this storage so it's technically straightforward to introduce such a mechanism if we need one.
Test Plan:
- Viewed buttons in web UI, checked enabled/disabled states.
- Clicked the buttons.
- Saw commands show up in the command queue.
- Saw some daemon stuff get scheduled.
- Ran CLI tools, saw commands get consumed and resources/leases release.
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14143
2015-09-23 16:42:08 +02:00
|
|
|
->addPropertyList($locks, pht('Slot Locks'))
|
|
|
|
|
->addPropertyList($commands, pht('Commands'));
|
2015-09-21 13:45:43 +02:00
|
|
|
|
2012-12-17 23:47:21 +01:00
|
|
|
return $this->buildApplicationPage(
|
2012-11-07 00:28:33 +01:00
|
|
|
array(
|
2012-12-17 23:47:21 +01:00
|
|
|
$crumbs,
|
2013-09-29 00:55:38 +02:00
|
|
|
$object_box,
|
2015-09-21 13:45:43 +02:00
|
|
|
$log_box,
|
2012-12-17 23:47:21 +01:00
|
|
|
),
|
2012-11-07 00:28:33 +01:00
|
|
|
array(
|
2015-09-21 13:45:43 +02:00
|
|
|
'title' => $title,
|
2012-11-07 00:28:33 +01:00
|
|
|
));
|
|
|
|
|
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
private function buildActionListView(DrydockLease $lease) {
|
Add a command queue to Drydock to manage lease/resource release
Summary:
Ref T9252. Broadly, Drydock currently races on releasing objects from the "active" state. To reproduce this:
- Scatter some sleep()s pretty much anywhere in the release code.
- Release several times from web UI or CLI in quick succession.
Resources or leases will execute some release code twice or otherwise do inconsistent things.
(I didn't chase down a detailed reproduction scenario for this since inspection of the code makes it clear that there are no meaningful locks or mechanisms preventing this.)
Instead, add a Harbormaster-style command queue to resources and leases. When something wants to do a release, it adds a command to the queue and schedules a worker. The workers acquire a lock, then try to consume commands from the queue.
This guarantees that only one process is responsible for writes to active resource/leases.
This is the last major step to giving resources and leases a single writer during all states:
- Resource, Unsaved: AllocatorWorker
- Resource, Pending: ResourceWorker (Possible rename to "Allocated?")
- Resource, Open: This diff, ResourceUpdateWorker. (Likely rename to "Active").
- Resource, Closed/Broken: Future destruction worker. (Likely rename to "Released" / "Broken"; maybe remove "Broken").
- Resource, Destroyed: No writes.
- Lease, Unsaved: Whatever wants the lease.
- Lease, Pending: AllocatorWorker
- Lease, Acquired: LeaseWorker
- Lease, Active: This diff, LeaseUpdateWorker.
- Lease, Released/Broken: Future destruction worker (Maybe remove "Broken"?)
- Lease, Expired: No writes. (Likely rename to "Destroyed").
In most phases, we can already guarantee that there is a single writer without doing any extra work. This is more complicated in the "Active" case because the release buttons on the web UI, the release tools on the CLI, the lease requestor itself, the garbage collector, and any other release process cleaning up related objects may try to effect a release. All of these could race one another (and, in many cases, race other processes from other phases because all of these get to act immediately) as this code is currently written. Using a queue here lets us make sure there's only a single writer in this phase.
One thing which is notable is that whatever acquires a lease **can not write to it**! It is never the writer once it queues the lease for activation. It can not write to any resources, either. And, likewise, Blueprints can not write to resources while acquiring or releasing leases.
We may need to provide a mechinism so that blueprints and/or resource/lease holders get to attach some storage to resources/leases for bookkeeping. For example, a blueprint might need to keep some kind of cache on a resource to help it manage state. But I think we can cross that bridge when we come to it, and nothing else would need to write to this storage so it's technically straightforward to introduce such a mechanism if we need one.
Test Plan:
- Viewed buttons in web UI, checked enabled/disabled states.
- Clicked the buttons.
- Saw commands show up in the command queue.
- Saw some daemon stuff get scheduled.
- Ran CLI tools, saw commands get consumed and resources/leases release.
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14143
2015-09-23 16:42:08 +02:00
|
|
|
$viewer = $this->getViewer();
|
|
|
|
|
|
2012-11-07 00:28:33 +01:00
|
|
|
$view = id(new PhabricatorActionListView())
|
Add a command queue to Drydock to manage lease/resource release
Summary:
Ref T9252. Broadly, Drydock currently races on releasing objects from the "active" state. To reproduce this:
- Scatter some sleep()s pretty much anywhere in the release code.
- Release several times from web UI or CLI in quick succession.
Resources or leases will execute some release code twice or otherwise do inconsistent things.
(I didn't chase down a detailed reproduction scenario for this since inspection of the code makes it clear that there are no meaningful locks or mechanisms preventing this.)
Instead, add a Harbormaster-style command queue to resources and leases. When something wants to do a release, it adds a command to the queue and schedules a worker. The workers acquire a lock, then try to consume commands from the queue.
This guarantees that only one process is responsible for writes to active resource/leases.
This is the last major step to giving resources and leases a single writer during all states:
- Resource, Unsaved: AllocatorWorker
- Resource, Pending: ResourceWorker (Possible rename to "Allocated?")
- Resource, Open: This diff, ResourceUpdateWorker. (Likely rename to "Active").
- Resource, Closed/Broken: Future destruction worker. (Likely rename to "Released" / "Broken"; maybe remove "Broken").
- Resource, Destroyed: No writes.
- Lease, Unsaved: Whatever wants the lease.
- Lease, Pending: AllocatorWorker
- Lease, Acquired: LeaseWorker
- Lease, Active: This diff, LeaseUpdateWorker.
- Lease, Released/Broken: Future destruction worker (Maybe remove "Broken"?)
- Lease, Expired: No writes. (Likely rename to "Destroyed").
In most phases, we can already guarantee that there is a single writer without doing any extra work. This is more complicated in the "Active" case because the release buttons on the web UI, the release tools on the CLI, the lease requestor itself, the garbage collector, and any other release process cleaning up related objects may try to effect a release. All of these could race one another (and, in many cases, race other processes from other phases because all of these get to act immediately) as this code is currently written. Using a queue here lets us make sure there's only a single writer in this phase.
One thing which is notable is that whatever acquires a lease **can not write to it**! It is never the writer once it queues the lease for activation. It can not write to any resources, either. And, likewise, Blueprints can not write to resources while acquiring or releasing leases.
We may need to provide a mechinism so that blueprints and/or resource/lease holders get to attach some storage to resources/leases for bookkeeping. For example, a blueprint might need to keep some kind of cache on a resource to help it manage state. But I think we can cross that bridge when we come to it, and nothing else would need to write to this storage so it's technically straightforward to introduce such a mechanism if we need one.
Test Plan:
- Viewed buttons in web UI, checked enabled/disabled states.
- Clicked the buttons.
- Saw commands show up in the command queue.
- Saw some daemon stuff get scheduled.
- Ran CLI tools, saw commands get consumed and resources/leases release.
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14143
2015-09-23 16:42:08 +02:00
|
|
|
->setUser($viewer)
|
2013-07-12 20:39:47 +02:00
|
|
|
->setObjectURI($this->getRequest()->getRequestURI())
|
2012-11-07 00:28:33 +01:00
|
|
|
->setObject($lease);
|
|
|
|
|
|
2012-12-15 00:42:58 +01:00
|
|
|
$id = $lease->getID();
|
|
|
|
|
|
Add a command queue to Drydock to manage lease/resource release
Summary:
Ref T9252. Broadly, Drydock currently races on releasing objects from the "active" state. To reproduce this:
- Scatter some sleep()s pretty much anywhere in the release code.
- Release several times from web UI or CLI in quick succession.
Resources or leases will execute some release code twice or otherwise do inconsistent things.
(I didn't chase down a detailed reproduction scenario for this since inspection of the code makes it clear that there are no meaningful locks or mechanisms preventing this.)
Instead, add a Harbormaster-style command queue to resources and leases. When something wants to do a release, it adds a command to the queue and schedules a worker. The workers acquire a lock, then try to consume commands from the queue.
This guarantees that only one process is responsible for writes to active resource/leases.
This is the last major step to giving resources and leases a single writer during all states:
- Resource, Unsaved: AllocatorWorker
- Resource, Pending: ResourceWorker (Possible rename to "Allocated?")
- Resource, Open: This diff, ResourceUpdateWorker. (Likely rename to "Active").
- Resource, Closed/Broken: Future destruction worker. (Likely rename to "Released" / "Broken"; maybe remove "Broken").
- Resource, Destroyed: No writes.
- Lease, Unsaved: Whatever wants the lease.
- Lease, Pending: AllocatorWorker
- Lease, Acquired: LeaseWorker
- Lease, Active: This diff, LeaseUpdateWorker.
- Lease, Released/Broken: Future destruction worker (Maybe remove "Broken"?)
- Lease, Expired: No writes. (Likely rename to "Destroyed").
In most phases, we can already guarantee that there is a single writer without doing any extra work. This is more complicated in the "Active" case because the release buttons on the web UI, the release tools on the CLI, the lease requestor itself, the garbage collector, and any other release process cleaning up related objects may try to effect a release. All of these could race one another (and, in many cases, race other processes from other phases because all of these get to act immediately) as this code is currently written. Using a queue here lets us make sure there's only a single writer in this phase.
One thing which is notable is that whatever acquires a lease **can not write to it**! It is never the writer once it queues the lease for activation. It can not write to any resources, either. And, likewise, Blueprints can not write to resources while acquiring or releasing leases.
We may need to provide a mechinism so that blueprints and/or resource/lease holders get to attach some storage to resources/leases for bookkeeping. For example, a blueprint might need to keep some kind of cache on a resource to help it manage state. But I think we can cross that bridge when we come to it, and nothing else would need to write to this storage so it's technically straightforward to introduce such a mechanism if we need one.
Test Plan:
- Viewed buttons in web UI, checked enabled/disabled states.
- Clicked the buttons.
- Saw commands show up in the command queue.
- Saw some daemon stuff get scheduled.
- Ran CLI tools, saw commands get consumed and resources/leases release.
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14143
2015-09-23 16:42:08 +02:00
|
|
|
$can_release = $lease->canRelease();
|
2015-09-28 18:35:26 +02:00
|
|
|
if ($lease->isReleasing()) {
|
|
|
|
|
$can_release = false;
|
|
|
|
|
}
|
|
|
|
|
|
Add a command queue to Drydock to manage lease/resource release
Summary:
Ref T9252. Broadly, Drydock currently races on releasing objects from the "active" state. To reproduce this:
- Scatter some sleep()s pretty much anywhere in the release code.
- Release several times from web UI or CLI in quick succession.
Resources or leases will execute some release code twice or otherwise do inconsistent things.
(I didn't chase down a detailed reproduction scenario for this since inspection of the code makes it clear that there are no meaningful locks or mechanisms preventing this.)
Instead, add a Harbormaster-style command queue to resources and leases. When something wants to do a release, it adds a command to the queue and schedules a worker. The workers acquire a lock, then try to consume commands from the queue.
This guarantees that only one process is responsible for writes to active resource/leases.
This is the last major step to giving resources and leases a single writer during all states:
- Resource, Unsaved: AllocatorWorker
- Resource, Pending: ResourceWorker (Possible rename to "Allocated?")
- Resource, Open: This diff, ResourceUpdateWorker. (Likely rename to "Active").
- Resource, Closed/Broken: Future destruction worker. (Likely rename to "Released" / "Broken"; maybe remove "Broken").
- Resource, Destroyed: No writes.
- Lease, Unsaved: Whatever wants the lease.
- Lease, Pending: AllocatorWorker
- Lease, Acquired: LeaseWorker
- Lease, Active: This diff, LeaseUpdateWorker.
- Lease, Released/Broken: Future destruction worker (Maybe remove "Broken"?)
- Lease, Expired: No writes. (Likely rename to "Destroyed").
In most phases, we can already guarantee that there is a single writer without doing any extra work. This is more complicated in the "Active" case because the release buttons on the web UI, the release tools on the CLI, the lease requestor itself, the garbage collector, and any other release process cleaning up related objects may try to effect a release. All of these could race one another (and, in many cases, race other processes from other phases because all of these get to act immediately) as this code is currently written. Using a queue here lets us make sure there's only a single writer in this phase.
One thing which is notable is that whatever acquires a lease **can not write to it**! It is never the writer once it queues the lease for activation. It can not write to any resources, either. And, likewise, Blueprints can not write to resources while acquiring or releasing leases.
We may need to provide a mechinism so that blueprints and/or resource/lease holders get to attach some storage to resources/leases for bookkeeping. For example, a blueprint might need to keep some kind of cache on a resource to help it manage state. But I think we can cross that bridge when we come to it, and nothing else would need to write to this storage so it's technically straightforward to introduce such a mechanism if we need one.
Test Plan:
- Viewed buttons in web UI, checked enabled/disabled states.
- Clicked the buttons.
- Saw commands show up in the command queue.
- Saw some daemon stuff get scheduled.
- Ran CLI tools, saw commands get consumed and resources/leases release.
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14143
2015-09-23 16:42:08 +02:00
|
|
|
$can_edit = PhabricatorPolicyFilter::hasCapability(
|
|
|
|
|
$viewer,
|
|
|
|
|
$lease,
|
|
|
|
|
PhabricatorPolicyCapability::CAN_EDIT);
|
2012-12-15 00:42:58 +01:00
|
|
|
|
|
|
|
|
$view->addAction(
|
|
|
|
|
id(new PhabricatorActionView())
|
|
|
|
|
->setName(pht('Release Lease'))
|
2014-05-13 16:45:39 +02:00
|
|
|
->setIcon('fa-times')
|
2012-12-15 00:42:58 +01:00
|
|
|
->setHref($this->getApplicationURI("/lease/{$id}/release/"))
|
|
|
|
|
->setWorkflow(true)
|
Add a command queue to Drydock to manage lease/resource release
Summary:
Ref T9252. Broadly, Drydock currently races on releasing objects from the "active" state. To reproduce this:
- Scatter some sleep()s pretty much anywhere in the release code.
- Release several times from web UI or CLI in quick succession.
Resources or leases will execute some release code twice or otherwise do inconsistent things.
(I didn't chase down a detailed reproduction scenario for this since inspection of the code makes it clear that there are no meaningful locks or mechanisms preventing this.)
Instead, add a Harbormaster-style command queue to resources and leases. When something wants to do a release, it adds a command to the queue and schedules a worker. The workers acquire a lock, then try to consume commands from the queue.
This guarantees that only one process is responsible for writes to active resource/leases.
This is the last major step to giving resources and leases a single writer during all states:
- Resource, Unsaved: AllocatorWorker
- Resource, Pending: ResourceWorker (Possible rename to "Allocated?")
- Resource, Open: This diff, ResourceUpdateWorker. (Likely rename to "Active").
- Resource, Closed/Broken: Future destruction worker. (Likely rename to "Released" / "Broken"; maybe remove "Broken").
- Resource, Destroyed: No writes.
- Lease, Unsaved: Whatever wants the lease.
- Lease, Pending: AllocatorWorker
- Lease, Acquired: LeaseWorker
- Lease, Active: This diff, LeaseUpdateWorker.
- Lease, Released/Broken: Future destruction worker (Maybe remove "Broken"?)
- Lease, Expired: No writes. (Likely rename to "Destroyed").
In most phases, we can already guarantee that there is a single writer without doing any extra work. This is more complicated in the "Active" case because the release buttons on the web UI, the release tools on the CLI, the lease requestor itself, the garbage collector, and any other release process cleaning up related objects may try to effect a release. All of these could race one another (and, in many cases, race other processes from other phases because all of these get to act immediately) as this code is currently written. Using a queue here lets us make sure there's only a single writer in this phase.
One thing which is notable is that whatever acquires a lease **can not write to it**! It is never the writer once it queues the lease for activation. It can not write to any resources, either. And, likewise, Blueprints can not write to resources while acquiring or releasing leases.
We may need to provide a mechinism so that blueprints and/or resource/lease holders get to attach some storage to resources/leases for bookkeeping. For example, a blueprint might need to keep some kind of cache on a resource to help it manage state. But I think we can cross that bridge when we come to it, and nothing else would need to write to this storage so it's technically straightforward to introduce such a mechanism if we need one.
Test Plan:
- Viewed buttons in web UI, checked enabled/disabled states.
- Clicked the buttons.
- Saw commands show up in the command queue.
- Saw some daemon stuff get scheduled.
- Ran CLI tools, saw commands get consumed and resources/leases release.
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14143
2015-09-23 16:42:08 +02:00
|
|
|
->setDisabled(!$can_release || !$can_edit));
|
2012-12-15 00:42:58 +01:00
|
|
|
|
2012-11-07 00:28:33 +01:00
|
|
|
return $view;
|
|
|
|
|
}
|
|
|
|
|
|
2013-10-11 16:53:56 +02:00
|
|
|
private function buildPropertyListView(
|
|
|
|
|
DrydockLease $lease,
|
|
|
|
|
PhabricatorActionListView $actions) {
|
2015-09-23 22:54:27 +02:00
|
|
|
$viewer = $this->getViewer();
|
2013-10-11 16:53:56 +02:00
|
|
|
|
|
|
|
|
$view = new PHUIPropertyListView();
|
|
|
|
|
$view->setActionList($actions);
|
2012-11-07 00:28:33 +01:00
|
|
|
|
|
|
|
|
$view->addProperty(
|
|
|
|
|
pht('Status'),
|
2015-09-24 05:48:51 +02:00
|
|
|
DrydockLeaseStatus::getNameForStatus($lease->getStatus()));
|
2012-11-07 00:28:33 +01:00
|
|
|
|
|
|
|
|
$view->addProperty(
|
|
|
|
|
pht('Resource Type'),
|
2013-01-29 20:01:47 +01:00
|
|
|
$lease->getResourceType());
|
2012-11-07 00:28:33 +01:00
|
|
|
|
2015-09-29 18:51:06 +02:00
|
|
|
$owner_phid = $lease->getOwnerPHID();
|
|
|
|
|
if ($owner_phid) {
|
|
|
|
|
$owner_display = $viewer->renderHandle($owner_phid);
|
|
|
|
|
} else {
|
|
|
|
|
$owner_display = phutil_tag('em', array(), pht('No Owner'));
|
|
|
|
|
}
|
|
|
|
|
$view->addProperty(pht('Owner'), $owner_display);
|
|
|
|
|
|
2015-09-24 13:19:27 +02:00
|
|
|
$resource_phid = $lease->getResourcePHID();
|
|
|
|
|
if ($resource_phid) {
|
|
|
|
|
$resource_display = $viewer->renderHandle($resource_phid);
|
2015-09-23 19:52:17 +02:00
|
|
|
} else {
|
2015-09-24 13:19:27 +02:00
|
|
|
$resource_display = phutil_tag('em', array(), pht('No Resource'));
|
2015-09-23 19:52:17 +02:00
|
|
|
}
|
2015-09-24 13:19:27 +02:00
|
|
|
$view->addProperty(pht('Resource'), $resource_display);
|
2012-11-07 00:28:33 +01:00
|
|
|
|
2015-09-23 22:54:27 +02:00
|
|
|
$until = $lease->getUntil();
|
|
|
|
|
if ($until) {
|
|
|
|
|
$until_display = phabricator_datetime($until, $viewer);
|
|
|
|
|
} else {
|
|
|
|
|
$until_display = phutil_tag('em', array(), pht('Never'));
|
|
|
|
|
}
|
|
|
|
|
$view->addProperty(pht('Expires'), $until_display);
|
|
|
|
|
|
2012-12-12 16:36:53 +01:00
|
|
|
$attributes = $lease->getAttributes();
|
|
|
|
|
if ($attributes) {
|
2015-09-19 20:29:01 +02:00
|
|
|
$view->addSectionHeader(
|
|
|
|
|
pht('Attributes'), 'fa-list-ul');
|
2012-12-12 16:36:53 +01:00
|
|
|
foreach ($attributes as $key => $value) {
|
2013-01-29 20:01:47 +01:00
|
|
|
$view->addProperty($key, $value);
|
2012-12-12 16:36:53 +01:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2012-11-07 00:28:33 +01:00
|
|
|
return $view;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
}
|
