Page MenuHomePhabricator

Merge the DrydockResource workers into a single worker
ClosedPublic

Authored by epriestley on Sep 30 2015, 7:44 PM.
Tags
None
Referenced Files
F13221429: D14201.diff
Sun, May 19, 2:46 AM
F13203946: D14201.diff
Wed, May 15, 12:14 AM
F13186615: D14201.diff
Sat, May 11, 3:44 AM
F13174262: D14201.id34300.diff
Tue, May 7, 10:30 PM
Unknown Object (File)
Tue, May 7, 6:54 AM
Unknown Object (File)
Mon, May 6, 6:58 PM
Unknown Object (File)
Fri, May 3, 4:11 AM
Unknown Object (File)
Mon, Apr 29, 4:09 PM
Subscribers
None

Details

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.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Merge the DrydockResource workers into a single worker.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
  • Remove some status checks which test conditions which are now unreachable.
chad edited edge metadata.
This revision is now accepted and ready to land.Sep 30 2015, 8:36 PM