Page MenuHomePhabricator

Fix a bug which prevented Conduit futures from having parallelism limited effectively
ClosedPublic

Authored by epriestley on Oct 12 2017, 7:18 PM.
Tags
None
Referenced Files
F18957025: D18704.diff
Wed, Nov 12, 11:13 PM
F18828317: D18704.diff
Fri, Oct 24, 3:44 PM
F18817801: D18704.id.diff
Tue, Oct 21, 6:33 PM
F18809618: D18704.id.diff
Sun, Oct 19, 3:35 PM
F18807864: D18704.diff
Sun, Oct 19, 3:32 AM
F18783344: D18704.id44897.diff
Oct 13 2025, 6:06 AM
F18745814: D18704.id44901.diff
Oct 3 2025, 9:36 AM
F18678843: D18704.id.diff
Sep 25 2025, 11:37 PM
Subscribers
None

Details

Summary

Ref T13008. Currently, ConduitClient calls isReady() on futures before returning them.

This is very old, and wrong: it makes the future activate and start working. In the case of ConduitFutures, which wrap HTTPFutures, it causes them to make an HTTP request.

Instead, return dormant futures without calling isReady(). Then the construct used by ArcanistFileUploader and many other pieces of code will limit parallelism properly:

$iterator = id(new FutureIterator($futures))
  ->limit(4);
foreach ($iterator as $future) {
 // ...
}

Also, fix up the --trace profiler integration so we get a log when the future actually starts, not when we create it.

Test Plan

Ran arc upload --trace ... with a bunch of files, fiddling with the limit(X). Saw 4 (default) and 1 properly throttle requests. Saw 200 unthrottle requests.

Diff Detail

Repository
rPHU libphutil
Branch
post6
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 18673
Build 25156: Run Core Tests
Build 25155: arc lint + arc unit