Skip to content

Commit 42c0f06

Browse files
author
epriestley
committed
Push feed publishing deeper into the task queue
Summary: Ref T2852. I want to model Asana integration as a response to feed events. Currently, we queue one feed event for each HTTP hook. Instead, always queue one feed event and then have it queue any necessary followup events (now, http hooks; soon, asana). Add a script to make it easy to reproducibly fire feed event publishing. Test Plan: Republished a feed event and verified it hit configured HTTP hooks correctly. $ ./bin/feed republish 5765774156541908292 --trace >>> [2] <connect> phabricator2_feed <<< [2] <connect> 1,660 us >>> [3] <query> SELECT story.* FROM `feed_storydata` story JOIN `feed_storyreference` ref ON ref.chronologicalKey = story.chronologicalKey WHERE (ref.chronologicalKey IN (5765774156541908292)) GROUP BY story.chronologicalKey ORDER BY story.chronologicalKey DESC <<< [3] <query> 595 us >>> [4] <connect> phabricator2_differential <<< [4] <connect> 760 us >>> [5] <query> SELECT * FROM `differential_revision` WHERE phid IN ('PHID-DREV-ywqmrj5zgkdloqh5p3c5') <<< [5] <query> 478 us >>> [6] <query> SELECT * FROM `differential_revision` WHERE phid IN ('PHID-DREV-ywqmrj5zgkdloqh5p3c5') <<< [6] <query> 449 us >>> [7] <connect> phabricator2_user <<< [7] <connect> 1,062 us >>> [8] <query> SELECT * FROM `user` WHERE phid in ('PHID-USER-lqiz3yd7wmk64ejugvov') <<< [8] <query> 540 us >>> [9] <connect> phabricator2_file <<< [9] <connect> 951 us >>> [10] <query> SELECT * FROM `file` WHERE phid IN ('PHID-FILE-gq6dlsysvxbn3dgwvky7') <<< [10] <query> 498 us >>> [11] <query> SELECT * FROM `user_status` WHERE userPHID IN ('PHID-USER-lqiz3yd7wmk64ejugvov') AND UNIX_TIMESTAMP() BETWEEN dateFrom AND dateTo <<< [11] <query> 507 us Republishing story... >>> [12] <query> SELECT story.* FROM `feed_storydata` story JOIN `feed_storyreference` ref ON ref.chronologicalKey = story.chronologicalKey WHERE (ref.chronologicalKey IN (5765774156541908292)) GROUP BY story.chronologicalKey ORDER BY story.chronologicalKey DESC <<< [12] <query> 685 us >>> [13] <query> SELECT * FROM `differential_revision` WHERE phid IN ('PHID-DREV-ywqmrj5zgkdloqh5p3c5') <<< [13] <query> 489 us >>> [14] <query> SELECT * FROM `differential_revision` WHERE phid IN ('PHID-DREV-ywqmrj5zgkdloqh5p3c5') <<< [14] <query> 512 us >>> [15] <query> SELECT * FROM `user` WHERE phid in ('PHID-USER-lqiz3yd7wmk64ejugvov') <<< [15] <query> 601 us >>> [16] <query> SELECT * FROM `file` WHERE phid IN ('PHID-FILE-gq6dlsysvxbn3dgwvky7') <<< [16] <query> 405 us >>> [17] <query> SELECT * FROM `user_status` WHERE userPHID IN ('PHID-USER-lqiz3yd7wmk64ejugvov') AND UNIX_TIMESTAMP() BETWEEN dateFrom AND dateTo <<< [17] <query> 551 us >>> [18] <query> SELECT story.* FROM `feed_storydata` story JOIN `feed_storyreference` ref ON ref.chronologicalKey = story.chronologicalKey WHERE (ref.chronologicalKey IN (5765774156541908292)) GROUP BY story.chronologicalKey ORDER BY story.chronologicalKey DESC <<< [18] <query> 507 us >>> [19] <query> SELECT * FROM `differential_revision` WHERE phid IN ('PHID-DREV-ywqmrj5zgkdloqh5p3c5') <<< [19] <query> 428 us >>> [20] <query> SELECT * FROM `differential_revision` WHERE phid IN ('PHID-DREV-ywqmrj5zgkdloqh5p3c5') <<< [20] <query> 419 us >>> [21] <query> SELECT * FROM `user` WHERE phid in ('PHID-USER-lqiz3yd7wmk64ejugvov') <<< [21] <query> 591 us >>> [22] <query> SELECT * FROM `file` WHERE phid IN ('PHID-FILE-gq6dlsysvxbn3dgwvky7') <<< [22] <query> 406 us >>> [23] <query> SELECT * FROM `user_status` WHERE userPHID IN ('PHID-USER-lqiz3yd7wmk64ejugvov') AND UNIX_TIMESTAMP() BETWEEN dateFrom AND dateTo <<< [23] <query> 593 us >>> [24] <http> http://127.0.0.1/derp/ <<< [24] <http> 746,157 us [2013-06-24 20:23:26] EXCEPTION: (HTTPFutureResponseStatusHTTP) [HTTP/500] Internal Server Error Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2852 Differential Revision: https://secure.phabricator.com/D6291
1 parent 0495eb1 commit 42c0f06

11 files changed

+213
-39
lines changed

‎bin/feed

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../scripts/setup/manage_feed.php

‎scripts/setup/manage_feed.php

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#!/usr/bin/env php
2+
<?php
3+
4+
$root = dirname(dirname(dirname(__FILE__)));
5+
require_once $root.'/scripts/__init_script__.php';
6+
7+
$args = new PhutilArgumentParser($argv);
8+
$args->setTagline('manage feed');
9+
$args->setSynopsis(<<<EOSYNOPSIS
10+
**feed** __command__ [__options__]
11+
Test and debug feed events.
12+
13+
EOSYNOPSIS
14+
);
15+
$args->parseStandardArguments();
16+
17+
$workflows = array(
18+
new PhabricatorFeedManagementRepublishWorkflow(),
19+
new PhutilHelpArgumentWorkflow(),
20+
);
21+
22+
$args->parseWorkflows($workflows);

‎src/__phutil_library_map__.php

+9-1
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,9 @@
575575
'DrydockSSHCommandInterface' => 'applications/drydock/interface/command/DrydockSSHCommandInterface.php',
576576
'DrydockWebrootInterface' => 'applications/drydock/interface/webroot/DrydockWebrootInterface.php',
577577
'DrydockWorkingCopyBlueprint' => 'applications/drydock/blueprint/DrydockWorkingCopyBlueprint.php',
578+
'FeedPublisherHTTPWorker' => 'applications/feed/worker/FeedPublisherHTTPWorker.php',
578579
'FeedPublisherWorker' => 'applications/feed/worker/FeedPublisherWorker.php',
580+
'FeedPushWorker' => 'applications/feed/worker/FeedPushWorker.php',
579581
'HarbormasterDAO' => 'applications/harbormaster/storage/HarbormasterDAO.php',
580582
'HarbormasterObject' => 'applications/harbormaster/storage/HarbormasterObject.php',
581583
'HarbormasterRunnerWorker' => 'applications/harbormaster/worker/HarbormasterRunnerWorker.php',
@@ -1051,6 +1053,8 @@
10511053
'PhabricatorFeedController' => 'applications/feed/controller/PhabricatorFeedController.php',
10521054
'PhabricatorFeedDAO' => 'applications/feed/storage/PhabricatorFeedDAO.php',
10531055
'PhabricatorFeedMainController' => 'applications/feed/controller/PhabricatorFeedMainController.php',
1056+
'PhabricatorFeedManagementRepublishWorkflow' => 'applications/feed/management/PhabricatorFeedManagementRepublishWorkflow.php',
1057+
'PhabricatorFeedManagementWorkflow' => 'applications/feed/management/PhabricatorFeedManagementWorkflow.php',
10541058
'PhabricatorFeedPublicStreamController' => 'applications/feed/controller/PhabricatorFeedPublicStreamController.php',
10551059
'PhabricatorFeedQuery' => 'applications/feed/PhabricatorFeedQuery.php',
10561060
'PhabricatorFeedStory' => 'applications/feed/story/PhabricatorFeedStory.php',
@@ -2456,7 +2460,9 @@
24562460
'DrydockSSHCommandInterface' => 'DrydockCommandInterface',
24572461
'DrydockWebrootInterface' => 'DrydockInterface',
24582462
'DrydockWorkingCopyBlueprint' => 'DrydockBlueprint',
2459-
'FeedPublisherWorker' => 'PhabricatorWorker',
2463+
'FeedPublisherHTTPWorker' => 'FeedPushWorker',
2464+
'FeedPublisherWorker' => 'FeedPushWorker',
2465+
'FeedPushWorker' => 'PhabricatorWorker',
24602466
'HarbormasterDAO' => 'PhabricatorLiskDAO',
24612467
'HarbormasterObject' => 'HarbormasterDAO',
24622468
'HarbormasterRunnerWorker' => 'PhabricatorWorker',
@@ -2929,6 +2935,8 @@
29292935
'PhabricatorFeedController' => 'PhabricatorController',
29302936
'PhabricatorFeedDAO' => 'PhabricatorLiskDAO',
29312937
'PhabricatorFeedMainController' => 'PhabricatorFeedController',
2938+
'PhabricatorFeedManagementRepublishWorkflow' => 'PhabricatorFeedManagementWorkflow',
2939+
'PhabricatorFeedManagementWorkflow' => 'PhutilArgumentWorkflow',
29322940
'PhabricatorFeedPublicStreamController' => 'PhabricatorFeedController',
29332941
'PhabricatorFeedQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
29342942
'PhabricatorFeedStory' => 'PhabricatorPolicyInterface',

‎src/applications/feed/PhabricatorFeedQuery.php

+24
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,18 @@ final class PhabricatorFeedQuery
44
extends PhabricatorCursorPagedPolicyAwareQuery {
55

66
private $filterPHIDs;
7+
private $chronologicalKeys;
78

89
public function setFilterPHIDs(array $phids) {
910
$this->filterPHIDs = $phids;
1011
return $this;
1112
}
1213

14+
public function withChronologicalKeys(array $keys) {
15+
$this->chronologicalKeys = $keys;
16+
return $this;
17+
}
18+
1319
protected function loadPage() {
1420

1521
$story_table = new PhabricatorFeedStoryData();
@@ -54,6 +60,24 @@ private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
5460
$this->filterPHIDs);
5561
}
5662

63+
if ($this->chronologicalKeys) {
64+
// NOTE: We want to use integers in the query so we can take advantage
65+
// of keys, but can't use %d on 32-bit systems. Make sure all the keys
66+
// are integers and then format them raw.
67+
68+
$keys = $this->chronologicalKeys;
69+
foreach ($keys as $key) {
70+
if (!ctype_digit($key)) {
71+
throw new Exception("Key '{$key}' is not a valid chronological key!");
72+
}
73+
}
74+
75+
$where[] = qsprintf(
76+
$conn_r,
77+
'ref.chronologicalKey IN (%Q)',
78+
implode(', ', $keys));
79+
}
80+
5781
$where[] = $this->buildPagingClause($conn_r);
5882

5983
return $this->formatWhereClause($where);

‎src/applications/feed/PhabricatorFeedStoryPublisher.php

+5-6
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,11 @@ public function publish() {
105105
$this->sendNotification($chrono_key);
106106
}
107107

108-
$uris = PhabricatorEnv::getEnvConfig('feed.http-hooks');
109-
foreach ($uris as $uri) {
110-
$task = PhabricatorWorker::scheduleTask(
111-
'FeedPublisherWorker',
112-
array('chrono_key' => $chrono_key, 'uri' => $uri));
113-
}
108+
PhabricatorWorker::scheduleTask(
109+
'FeedPublisherWorker',
110+
array(
111+
'key' => $chrono_key,
112+
));
114113

115114
return $story;
116115
}

‎src/applications/feed/management/PhabricatorFeedManagementRepublishWorkflow.php

+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
<?php
2+
3+
final class PhabricatorFeedManagementRepublishWorkflow
4+
extends PhabricatorFeedManagementWorkflow {
5+
6+
protected function didConstruct() {
7+
$this
8+
->setName('republish')
9+
->setExamples('**republish** __story_key__')
10+
->setSynopsis(
11+
pht(
12+
'Republish a feed event to all consumers.'))
13+
->setArguments(
14+
array(
15+
array(
16+
'name' => 'key',
17+
'wildcard' => true,
18+
),
19+
));
20+
}
21+
22+
public function execute(PhutilArgumentParser $args) {
23+
$console = PhutilConsole::getConsole();
24+
$viewer = PhabricatorUser::getOmnipotentUser();
25+
26+
$key = $args->getArg('key');
27+
if (count($key) < 1) {
28+
throw new PhutilArgumentUsageException(
29+
pht("Specify a story key to republish."));
30+
} else if (count($key) > 1) {
31+
throw new PhutilArgumentUsageException(
32+
pht("Specify exactly one story key to republish."));
33+
}
34+
$key = head($key);
35+
36+
$story = id(new PhabricatorFeedQuery())
37+
->setViewer($viewer)
38+
->withChronologicalKeys(array($key))
39+
->executeOne();
40+
41+
if (!$story) {
42+
throw new PhutilArgumentUsageException(
43+
pht('No story exists with key "%s"!', $key));
44+
}
45+
46+
$console->writeOut("%s\n", pht("Republishing story..."));
47+
48+
PhabricatorWorker::setRunAllTasksInProcess(true);
49+
50+
PhabricatorWorker::scheduleTask(
51+
'FeedPublisherWorker',
52+
array(
53+
'key' => $key,
54+
));
55+
56+
$console->writeOut("%s\n", pht("Done."));
57+
58+
return 0;
59+
}
60+
61+
}

‎src/applications/feed/management/PhabricatorFeedManagementWorkflow.php

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
abstract class PhabricatorFeedManagementWorkflow
4+
extends PhutilArgumentWorkflow {
5+
6+
final public function isExecutable() {
7+
return true;
8+
}
9+
10+
}

‎src/applications/feed/worker/FeedPublisherHTTPWorker.php

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php
2+
3+
final class FeedPublisherHTTPWorker extends FeedPushWorker {
4+
5+
protected function doWork() {
6+
$story = $this->loadFeedStory();
7+
$data = $story->getStoryData();
8+
9+
$uri = idx($this->getTaskData(), 'uri');
10+
11+
$post_data = array(
12+
'storyID' => $data->getID(),
13+
'storyType' => $data->getStoryType(),
14+
'storyData' => $data->getStoryData(),
15+
'storyAuthorPHID' => $data->getAuthorPHID(),
16+
'epoch' => $data->getEpoch(),
17+
);
18+
19+
id(new HTTPFuture($uri, $post_data))
20+
->setMethod('POST')
21+
->setTimeout(30)
22+
->resolvex();
23+
}
24+
25+
public function getWaitBeforeRetry(PhabricatorWorkerTask $task) {
26+
return max($task->getFailureCount(), 1) * 60;
27+
}
28+
29+
}

‎src/applications/feed/worker/FeedPublisherWorker.php

+11-28
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,21 @@
11
<?php
22

3-
final class FeedPublisherWorker extends PhabricatorWorker {
3+
final class FeedPublisherWorker extends FeedPushWorker {
44

55
protected function doWork() {
6-
$task_data = $this->getTaskData();
7-
$chrono_key = $task_data['chrono_key'];
8-
$uri = $task_data['uri'];
9-
10-
$story = id(new PhabricatorFeedStoryData())
11-
->loadOneWhere('chronologicalKey = %s', $chrono_key);
12-
13-
if (!$story) {
14-
throw new PhabricatorWorkerPermanentFailureException(
15-
'Feed story was deleted.'
16-
);
6+
$story = $this->loadFeedStory();
7+
8+
$uris = PhabricatorEnv::getEnvConfig('feed.http-hooks');
9+
foreach ($uris as $uri) {
10+
PhabricatorWorker::scheduleTask(
11+
'FeedPublisherHTTPWorker',
12+
array(
13+
'key' => $story->getChronologicalKey(),
14+
'uri' => $uri,
15+
));
1716
}
1817

19-
$data = array(
20-
'storyID' => $story->getID(),
21-
'storyType' => $story->getStoryType(),
22-
'storyData' => $story->getStoryData(),
23-
'storyAuthorPHID' => $story->getAuthorPHID(),
24-
'epoch' => $story->getEpoch(),
25-
);
26-
27-
id(new HTTPFuture($uri, $data))
28-
->setMethod('POST')
29-
->setTimeout(30)
30-
->resolvex();
31-
3218
}
3319

34-
public function getWaitBeforeRetry(PhabricatorWorkerTask $task) {
35-
return max($task->getFailureCount(), 1) * 60;
36-
}
3720

3821
}

‎src/applications/feed/worker/FeedPushWorker.php

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
abstract class FeedPushWorker extends PhabricatorWorker {
4+
5+
protected function loadFeedStory() {
6+
$task_data = $this->getTaskData();
7+
$key = $task_data['key'];
8+
9+
$story = id(new PhabricatorFeedQuery())
10+
->setViewer(PhabricatorUser::getOmnipotentUser())
11+
->withChronologicalKeys(array($key))
12+
->executeOne();
13+
14+
if (!$story) {
15+
throw new PhabricatorWorkerPermanentFailureException(
16+
'Feed story does not exist..');
17+
}
18+
19+
return $story;
20+
}
21+
22+
}

‎src/infrastructure/daemon/workers/PhabricatorWorker.php

+19-4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
abstract class PhabricatorWorker {
99

1010
private $data;
11+
private static $runAllTasksInProcess = false;
1112

1213

1314
/* -( Configuring Retries and Failures )----------------------------------- */
@@ -85,10 +86,15 @@ final public function executeTask() {
8586
}
8687

8788
final public static function scheduleTask($task_class, $data) {
88-
return id(new PhabricatorWorkerActiveTask())
89-
->setTaskClass($task_class)
90-
->setData($data)
91-
->save();
89+
if (self::$runAllTasksInProcess) {
90+
$worker = newv($task_class, array($data));
91+
$worker->doWork();
92+
} else {
93+
return id(new PhabricatorWorkerActiveTask())
94+
->setTaskClass($task_class)
95+
->setData($data)
96+
->save();
97+
}
9298
}
9399

94100

@@ -154,4 +160,13 @@ public function renderForDisplay() {
154160
return phutil_tag('pre', array(), $data);
155161
}
156162

163+
/**
164+
* Set this flag to execute scheduled tasks synchronously, in the same
165+
* process. This is useful for debugging, and otherwise dramatically worse
166+
* in every way imaginable.
167+
*/
168+
public static function setRunAllTasksInProcess($all) {
169+
self::$runAllTasksInProcess = $all;
170+
}
171+
157172
}

0 commit comments

Comments
 (0)