Page MenuHomePhabricator

Fix an issue where Drydock followup tasks would not queue if the main task failed
ClosedPublic

Authored by epriestley on Dec 18 2015, 3:24 PM.
Tags
None
Referenced Files
F13204132: D14818.diff
Wed, May 15, 12:21 AM
F13199707: D14818.id35826.diff
Mon, May 13, 5:18 PM
F13186927: D14818.diff
Sat, May 11, 4:13 AM
F13176146: D14818.id35826.diff
Wed, May 8, 10:59 AM
Unknown Object (File)
Tue, May 7, 7:17 AM
Unknown Object (File)
Fri, May 3, 8:33 AM
Unknown Object (File)
Fri, May 3, 5:48 AM
Unknown Object (File)
Mon, Apr 29, 4:49 PM
Subscribers
None

Details

Summary

Ref T9994. This fixes the first issue discussed on that task, which is that when a merge fails after "arc land", we would not clean up all the leases properly.

Specifically, when a merge fails, we use queueTask() to schedule a followup task. This followup destroys the lease and frees the underlying resource.

However, the default behavior of queueTask() is to not queue tasks if the parent task fails. This is a reasonable, safe behavior that was originally introduced in D8774, where it kept us from sending too much mail if a task did "send some mail" and then failed a little later on and got retried.

Since I think the default behavior is correct, I just special cased the behavior for Drydock to make it queue even on failure. These are the only types of followup tasks we currently want to queue on main task failure.

(It's possible that future Blueprints might want some kind of more specialized behavior, where some tasks queue only on success, but we can cross that bridge when we come to it.)

Test Plan
  • See T9994#149878 for test case setup.
  • I ran that test case again with this patch, and saw the followup task queue properly in the --trace log, a correspoinding update task show up in /daemon/, and the lease get destroyed when I ran it a moment later.

destroyed.png (437×1 px, 90 KB)

Diff Detail

Repository
rP Phabricator
Branch
drydock1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 9643
Build 11547: Run Core Tests
Build 11546: arc lint + arc unit

Event Timeline

  • Remove unrelated example code.
chad edited edge metadata.
This revision is now accepted and ready to land.Dec 18 2015, 4:12 PM
This revision was automatically updated to reflect the committed changes.