mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-18 12:52:42 +01:00
Allow Maniphest to scale to a massive size
Summary: Maniphest is missing some keys and some query strategy which will make it cumbersome to manage more than a few tens of thousands of tasks. Test Plan: Handily manipulated 100k-scale task groups. Maniphest takes about 250ms to select and render pages of 1,000 tasks and has no problem paging and filtering them, etc. We should be good to scale to multiple millions of tasks with these changes. Reviewed By: gc3 Reviewers: fratrik, jungejason, aran, tuomaspelkonen, gc3 Commenters: jungejason CC: anjali, aran, epriestley, gc3, jungejason Differential Revision: 534
This commit is contained in:
parent
e0e6ec9117
commit
4469ef8f30
8 changed files with 162 additions and 50 deletions
20
resources/sql/patches/050.taskdenormal.sql
Normal file
20
resources/sql/patches/050.taskdenormal.sql
Normal file
|
@ -0,0 +1,20 @@
|
|||
ALTER TABLE phabricator_maniphest.maniphest_task
|
||||
ADD ownerOrdering varchar(64);
|
||||
|
||||
ALTER TABLE phabricator_maniphest.maniphest_task
|
||||
ADD UNIQUE KEY (phid);
|
||||
|
||||
ALTER TABLE phabricator_maniphest.maniphest_task
|
||||
ADD KEY (priority, status);
|
||||
|
||||
ALTER TABLE phabricator_maniphest.maniphest_task
|
||||
ADD KEY (status);
|
||||
|
||||
ALTER TABLE phabricator_maniphest.maniphest_task
|
||||
ADD KEY (ownerPHID, status);
|
||||
|
||||
ALTER TABLE phabricator_maniphest.maniphest_task
|
||||
ADD KEY (authorPHID, status);
|
||||
|
||||
ALTER TABLE phabricator_maniphest.maniphest_task
|
||||
ADD KEY (ownerOrdering);
|
|
@ -72,7 +72,7 @@ celerity_register_resource_map(array(
|
|||
),
|
||||
'aphront-list-filter-view-css' =>
|
||||
array(
|
||||
'uri' => '/res/e6cff171/rsrc/css/aphront/list-filter-view.css',
|
||||
'uri' => '/res/0f5ddaba/rsrc/css/aphront/list-filter-view.css',
|
||||
'type' => 'css',
|
||||
'requires' =>
|
||||
array(
|
||||
|
@ -893,7 +893,7 @@ celerity_register_resource_map(array(
|
|||
),
|
||||
'maniphest-task-summary-css' =>
|
||||
array(
|
||||
'uri' => '/res/0bacdd7f/rsrc/css/application/maniphest/task-summary.css',
|
||||
'uri' => '/res/e86389c4/rsrc/css/application/maniphest/task-summary.css',
|
||||
'type' => 'css',
|
||||
'requires' =>
|
||||
array(
|
||||
|
@ -1132,7 +1132,24 @@ celerity_register_resource_map(array(
|
|||
'uri' => '/res/pkg/2892314d/typeahead.pkg.js',
|
||||
'type' => 'js',
|
||||
),
|
||||
'cfafcff7' =>
|
||||
55967526 =>
|
||||
array (
|
||||
'name' => 'differential.pkg.css',
|
||||
'symbols' =>
|
||||
array (
|
||||
0 => 'differential-core-view-css',
|
||||
1 => 'differential-changeset-view-css',
|
||||
2 => 'differential-revision-detail-css',
|
||||
3 => 'differential-revision-history-css',
|
||||
4 => 'differential-table-of-contents-css',
|
||||
5 => 'differential-revision-comment-css',
|
||||
6 => 'differential-revision-add-comment-css',
|
||||
7 => 'differential-revision-comment-list-css',
|
||||
),
|
||||
'uri' => '/res/pkg/55967526/differential.pkg.css',
|
||||
'type' => 'css',
|
||||
),
|
||||
'8e9024dc' =>
|
||||
array (
|
||||
'name' => 'core.pkg.css',
|
||||
'symbols' =>
|
||||
|
@ -1153,7 +1170,7 @@ celerity_register_resource_map(array(
|
|||
13 => 'phabricator-remarkup-css',
|
||||
14 => 'syntax-highlighting-css',
|
||||
),
|
||||
'uri' => '/res/pkg/cfafcff7/core.pkg.css',
|
||||
'uri' => '/res/pkg/8e9024dc/core.pkg.css',
|
||||
'type' => 'css',
|
||||
),
|
||||
'da416e1c' =>
|
||||
|
@ -1205,35 +1222,18 @@ celerity_register_resource_map(array(
|
|||
'uri' => '/res/pkg/f1d27e2a/workflow.pkg.js',
|
||||
'type' => 'js',
|
||||
),
|
||||
55967526 =>
|
||||
array (
|
||||
'name' => 'differential.pkg.css',
|
||||
'symbols' =>
|
||||
array (
|
||||
0 => 'differential-core-view-css',
|
||||
1 => 'differential-changeset-view-css',
|
||||
2 => 'differential-revision-detail-css',
|
||||
3 => 'differential-revision-history-css',
|
||||
4 => 'differential-table-of-contents-css',
|
||||
5 => 'differential-revision-comment-css',
|
||||
6 => 'differential-revision-add-comment-css',
|
||||
7 => 'differential-revision-comment-list-css',
|
||||
),
|
||||
'uri' => '/res/pkg/55967526/differential.pkg.css',
|
||||
'type' => 'css',
|
||||
),
|
||||
),
|
||||
'reverse' =>
|
||||
array (
|
||||
'aphront-crumbs-view-css' => 'cfafcff7',
|
||||
'aphront-dialog-view-css' => 'cfafcff7',
|
||||
'aphront-form-view-css' => 'cfafcff7',
|
||||
'aphront-list-filter-view-css' => 'cfafcff7',
|
||||
'aphront-panel-view-css' => 'cfafcff7',
|
||||
'aphront-side-nav-view-css' => 'cfafcff7',
|
||||
'aphront-table-view-css' => 'cfafcff7',
|
||||
'aphront-tokenizer-control-css' => 'cfafcff7',
|
||||
'aphront-typeahead-control-css' => 'cfafcff7',
|
||||
'aphront-crumbs-view-css' => '8e9024dc',
|
||||
'aphront-dialog-view-css' => '8e9024dc',
|
||||
'aphront-form-view-css' => '8e9024dc',
|
||||
'aphront-list-filter-view-css' => '8e9024dc',
|
||||
'aphront-panel-view-css' => '8e9024dc',
|
||||
'aphront-side-nav-view-css' => '8e9024dc',
|
||||
'aphront-table-view-css' => '8e9024dc',
|
||||
'aphront-tokenizer-control-css' => '8e9024dc',
|
||||
'aphront-typeahead-control-css' => '8e9024dc',
|
||||
'differential-changeset-view-css' => '55967526',
|
||||
'differential-core-view-css' => '55967526',
|
||||
'differential-revision-add-comment-css' => '55967526',
|
||||
|
@ -1270,13 +1270,13 @@ celerity_register_resource_map(array(
|
|||
'javelin-util' => 'db95a6d0',
|
||||
'javelin-vector' => 'db95a6d0',
|
||||
'javelin-workflow' => 'f1d27e2a',
|
||||
'phabricator-core-buttons-css' => 'cfafcff7',
|
||||
'phabricator-core-css' => 'cfafcff7',
|
||||
'phabricator-directory-css' => 'cfafcff7',
|
||||
'phabricator-core-buttons-css' => '8e9024dc',
|
||||
'phabricator-core-css' => '8e9024dc',
|
||||
'phabricator-directory-css' => '8e9024dc',
|
||||
'phabricator-keyboard-shortcut' => 'f1d27e2a',
|
||||
'phabricator-keyboard-shortcut-manager' => 'f1d27e2a',
|
||||
'phabricator-remarkup-css' => 'cfafcff7',
|
||||
'phabricator-standard-page-view' => 'cfafcff7',
|
||||
'syntax-highlighting-css' => 'cfafcff7',
|
||||
'phabricator-remarkup-css' => '8e9024dc',
|
||||
'phabricator-standard-page-view' => '8e9024dc',
|
||||
'syntax-highlighting-css' => '8e9024dc',
|
||||
),
|
||||
));
|
||||
|
|
|
@ -18,6 +18,8 @@
|
|||
|
||||
class ManiphestTaskListController extends ManiphestController {
|
||||
|
||||
const DEFAULT_PAGE_SIZE = 1000;
|
||||
|
||||
private $view;
|
||||
|
||||
public function willProcessRequest(array $data) {
|
||||
|
@ -90,12 +92,17 @@ class ManiphestTaskListController extends ManiphestController {
|
|||
|
||||
$view_phid = nonempty($request->getStr('phid'), $user->getPHID());
|
||||
|
||||
list($tasks, $handles) = $this->loadTasks(
|
||||
$page = $request->getInt('page');
|
||||
$page_size = self::DEFAULT_PAGE_SIZE;
|
||||
|
||||
list($tasks, $handles, $total_count) = $this->loadTasks(
|
||||
$view_phid,
|
||||
array(
|
||||
'status' => $status_map,
|
||||
'group' => $grouping,
|
||||
'order' => $order,
|
||||
'offset' => $page,
|
||||
'limit' => $page_size,
|
||||
));
|
||||
|
||||
|
||||
|
@ -158,6 +165,25 @@ class ManiphestTaskListController extends ManiphestController {
|
|||
'No matching tasks.'.
|
||||
'</h1>');
|
||||
} else {
|
||||
$pager = new AphrontPagerView();
|
||||
$pager->setURI($request->getRequestURI(), 'page');
|
||||
$pager->setPageSize($page_size);
|
||||
$pager->setOffset($page);
|
||||
$pager->setCount($total_count);
|
||||
|
||||
$cur = ($pager->getOffset() + 1);
|
||||
$max = min($pager->getOffset() + $page_size, $total_count);
|
||||
$tot = $total_count;
|
||||
|
||||
$cur = number_format($cur);
|
||||
$max = number_format($max);
|
||||
$tot = number_format($tot);
|
||||
|
||||
$nav->appendChild(
|
||||
'<div class="maniphest-total-result-count">'.
|
||||
"Displaying tasks {$cur} - {$max} of {$tot}.".
|
||||
'</div>');
|
||||
|
||||
foreach ($tasks as $group => $list) {
|
||||
$task_list = new ManiphestTaskListView();
|
||||
$task_list->setUser($user);
|
||||
|
@ -171,6 +197,8 @@ class ManiphestTaskListController extends ManiphestController {
|
|||
'</h1>');
|
||||
$nav->appendChild($task_list);
|
||||
}
|
||||
|
||||
$nav->appendChild($pager);
|
||||
}
|
||||
|
||||
return $this->buildStandardPageResponse(
|
||||
|
@ -224,23 +252,63 @@ class ManiphestTaskListController extends ManiphestController {
|
|||
break;
|
||||
}
|
||||
|
||||
switch ($dict['order']) {
|
||||
$order = array();
|
||||
switch ($dict['group']) {
|
||||
case 'priority':
|
||||
$order = 'priority DESC, dateModified DESC';
|
||||
$order[] = 'priority';
|
||||
break;
|
||||
case 'created':
|
||||
$order = 'id DESC';
|
||||
case 'owner':
|
||||
$order[] = 'ownerOrdering';
|
||||
break;
|
||||
default:
|
||||
$order = 'dateModified DESC';
|
||||
case 'status':
|
||||
$order[] = 'status';
|
||||
break;
|
||||
}
|
||||
|
||||
$sql = "({$status_clause}) AND ({$extra_clause}) ORDER BY {$order}";
|
||||
switch ($dict['order']) {
|
||||
case 'priority':
|
||||
$order[] = 'priority';
|
||||
$order[] = 'dateModified';
|
||||
break;
|
||||
case 'created':
|
||||
$order[] = 'id';
|
||||
break;
|
||||
default:
|
||||
$order[] = 'dateModified';
|
||||
break;
|
||||
}
|
||||
|
||||
array_unshift($argv, $sql);
|
||||
$order = array_unique($order);
|
||||
|
||||
$data = call_user_func_array(array($task, 'loadAllWhere'), $argv);
|
||||
foreach ($order as $k => $column) {
|
||||
switch ($column) {
|
||||
case 'ownerOrdering':
|
||||
$order[$k] = "{$column} ASC";
|
||||
break;
|
||||
default:
|
||||
$order[$k] = "{$column} DESC";
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
$order = implode(', ', $order);
|
||||
|
||||
$offset = (int)idx($dict, 'offset', 0);
|
||||
$limit = (int)idx($dict, 'limit', self::DEFAULT_PAGE_SIZE);
|
||||
|
||||
$sql = "SELECT SQL_CALC_FOUND_ROWS * FROM %T WHERE ".
|
||||
"({$status_clause}) AND ({$extra_clause}) ORDER BY {$order} ".
|
||||
"LIMIT {$offset}, {$limit}";
|
||||
|
||||
array_unshift($argv, $task->getTableName());
|
||||
|
||||
$conn = $task->establishConnection('r');
|
||||
$data = vqueryfx_all($conn, $sql, $argv);
|
||||
|
||||
$total_row_count = queryfx_one($conn, 'SELECT FOUND_ROWS() N');
|
||||
$total_row_count = $total_row_count['N'];
|
||||
|
||||
$data = $task->loadAllFromArray($data);
|
||||
|
||||
$handle_phids = mpull($data, 'getOwnerPHID');
|
||||
$handle_phids[] = $view_phid;
|
||||
|
@ -252,9 +320,15 @@ class ManiphestTaskListController extends ManiphestController {
|
|||
$data = mgroup($data, 'getPriority');
|
||||
krsort($data);
|
||||
|
||||
// If we have invalid priorities, they'll all map to "???". Merge
|
||||
// arrays to prevent them from overwriting each other.
|
||||
|
||||
$out = array();
|
||||
foreach ($data as $pri => $tasks) {
|
||||
$out[ManiphestTaskPriority::getTaskPriorityName($pri)] = $tasks;
|
||||
$out[ManiphestTaskPriority::getTaskPriorityName($pri)][] = $tasks;
|
||||
}
|
||||
foreach ($out as $pri => $tasks) {
|
||||
$out[$pri] = array_mergev($tasks);
|
||||
}
|
||||
$data = $out;
|
||||
|
||||
|
@ -297,7 +371,7 @@ class ManiphestTaskListController extends ManiphestController {
|
|||
break;
|
||||
}
|
||||
|
||||
return array($data, $handles);
|
||||
return array($data, $handles, $total_row_count);
|
||||
}
|
||||
|
||||
public function renderStatusLinks() {
|
||||
|
|
|
@ -14,6 +14,8 @@ phutil_require_module('phabricator', 'applications/maniphest/storage/task');
|
|||
phutil_require_module('phabricator', 'applications/maniphest/view/tasklist');
|
||||
phutil_require_module('phabricator', 'applications/phid/handle/data');
|
||||
phutil_require_module('phabricator', 'infrastructure/celerity/api');
|
||||
phutil_require_module('phabricator', 'storage/queryfx');
|
||||
phutil_require_module('phabricator', 'view/control/pager');
|
||||
phutil_require_module('phabricator', 'view/form/base');
|
||||
phutil_require_module('phabricator', 'view/form/control/togglebuttons');
|
||||
phutil_require_module('phabricator', 'view/form/control/tokenizer');
|
||||
|
|
|
@ -89,6 +89,13 @@ class ManiphestTransactionEditor {
|
|||
$task->setStatus($new);
|
||||
break;
|
||||
case ManiphestTransactionType::TYPE_OWNER:
|
||||
if ($new) {
|
||||
$handles = id(new PhabricatorObjectHandleData(array($new)))
|
||||
->loadHandles();
|
||||
$task->setOwnerOrdering($handles[$new]->getName());
|
||||
} else {
|
||||
$task->setOwnerOrdering(null);
|
||||
}
|
||||
$task->setOwnerPHID($new);
|
||||
break;
|
||||
case ManiphestTransactionType::TYPE_CCS:
|
||||
|
|
|
@ -34,6 +34,8 @@ class ManiphestTask extends ManiphestDAO {
|
|||
protected $attached = array();
|
||||
protected $projectPHIDs = array();
|
||||
|
||||
protected $ownerOrdering;
|
||||
|
||||
public function getConfiguration() {
|
||||
return array(
|
||||
self::CONFIG_AUX_PHID => true,
|
||||
|
|
|
@ -6,7 +6,7 @@
|
|||
background: #f6f6f6;
|
||||
border-bottom: 1px solid #bbbbbb;
|
||||
width: 100%;
|
||||
margin-bottom: 2em;
|
||||
margin-bottom: 1em;
|
||||
}
|
||||
|
||||
.aphront-list-filter-view-buttons {
|
||||
|
|
|
@ -82,6 +82,13 @@
|
|||
|
||||
.maniphest-task-group-header {
|
||||
font-size: 18px;
|
||||
margin: 1.5em 14px 0;
|
||||
margin: 1em 14px 0;
|
||||
border-bottom: 1px solid #dddddd;
|
||||
}
|
||||
|
||||
.maniphest-total-result-count {
|
||||
text-align: right;
|
||||
padding-right: 2em;
|
||||
font-size: 11px;
|
||||
color: #666666;
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue