Skip to content

Commit

Permalink
tec: Improve the performance of news refreshing
Browse files Browse the repository at this point in the history
  • Loading branch information
marienfressinaud committed Oct 2, 2024
1 parent 7d550fa commit 00ac09b
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 26 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

namespace App\migrations;

class Migration202410020002AddCreatedAtIndexOnLinksToCollections
{
public function migrate(): bool
{
$database = \Minz\Database::get();

$database->exec(<<<'SQL'
CREATE INDEX idx_links_to_collections_created_at ON links_to_collections(created_at);
SQL);

return true;
}

public function rollback(): bool
{
$database = \Minz\Database::get();

$database->exec(<<<'SQL'
DROP INDEX idx_links_to_collections_created_at;
SQL);

return true;
}
}
46 changes: 42 additions & 4 deletions src/models/dao/links/NewsQueries.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,22 @@ trait NewsQueries
*
* @return self[]
*/
public static function listFromFollowedCollections(string $user_id): array
public static function listFromFollowedCollections(string $user_id, int $max): array
{
$values = [
':user_id' => $user_id,
':until_hard_limit' => \Minz\Time::ago(6, 'months')->format(Database\Column::DATETIME_FORMAT),
':until_strict' => \Minz\Time::ago(1, 'day')->format(Database\Column::DATETIME_FORMAT),
':until_normal' => \Minz\Time::ago(1, 'week')->format(Database\Column::DATETIME_FORMAT),
];

$sql = <<<SQL
SELECT l.*, lc.created_at AS published_at, 'collection' AS source_news_type, c.id AS source_news_resource_id
SELECT
l.url_hash,
l.*,
lc.created_at AS published_at,
'collection' AS source_news_type,
c.id AS source_news_resource_id
FROM collections c, links_to_collections lc, followed_collections fc, links l
WHERE fc.user_id = :user_id
Expand All @@ -47,20 +53,52 @@ public static function listFromFollowedCollections(string $user_id): array
AND l.user_id != :user_id
AND lc.created_at >= :until_hard_limit
AND (
(fc.time_filter = 'strict' AND lc.created_at >= :until_strict) OR
(fc.time_filter = 'normal' AND lc.created_at >= :until_normal) OR
(fc.time_filter = 'all' AND lc.created_at >= fc.created_at - INTERVAL '1 week')
)
ORDER BY published_at DESC, l.id
-- This order by is not very performant, but it gives a chance to
-- get links that wouldn't show up otherwise. Indeed, if the first
-- 1000 links were removed from the news, even if the next links
-- had to be returned, nothing would be returned from the method.
ORDER BY random()
LIMIT 1000
SQL;

$database = Database::get();
$statement = $database->prepare($sql);
$statement->execute($values);

return self::fromDatabaseRows($statement->fetchAll());
// Get the results indexed by the url_hash (i.e. the first column)
$results = $statement->fetchAll(\PDO::FETCH_UNIQUE);

// Remove the links that must be excluded from the news
$excluded_hashes = self::listHashesExcludedFromNews($user_id);
$results = array_diff_key($results, $excluded_hashes);

// Limit the set of results
$results = array_slice($results, 0, $max);

// Transform the raw results to Links
$links = self::fromDatabaseRows($results);

// TODO sort before array_slice
// Sort the links
usort($links, function ($link1, $link2) {
$comparison = $link1->published_at <=> $link2->published_at;

if ($comparison === 0) {
return $link1->id <=> $link2->id;
}

return $comparison;
});

return $links;
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ CREATE TABLE links_to_collections (

CREATE UNIQUE INDEX idx_links_to_collections ON links_to_collections(link_id, collection_id);
CREATE INDEX idx_links_to_collections_collection_id ON links_to_collections(collection_id);
CREATE INDEX idx_links_to_collections_created_at ON links_to_collections(created_at);

CREATE TABLE followed_collections (
id BIGSERIAL PRIMARY KEY,
Expand Down
21 changes: 1 addition & 20 deletions src/services/NewsPicker.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,6 @@ public function __construct(models\User $user)
*/
public function pick(int $max = 25): array
{
$excluded_hashes = models\Link::listHashesExcludedFromNews($this->user->id);
$links_from_followed = models\Link::listFromFollowedCollections($this->user->id);

$links = [];

foreach ($links_from_followed as $link) {
$hash = $link->url_hash;

if (isset($excluded_hashes[$hash])) {
continue;
}

$links[$hash] = $link;

if (count($links) >= $max) {
break;
}
}

return array_values($links);
return models\Link::listFromFollowedCollections($this->user->id, $max);
}
}
4 changes: 2 additions & 2 deletions tests/services/NewsPickerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public function testPickRespectsFromFollowedIfOldLinksButWithTimeFilterAll(): vo
$now = $this->fake('dateTime');
$this->freeze($now);
/** @var int */
$days_ago = $this->fake('numberBetween', 8, 9999);
$days_ago = $this->fake('numberBetween', 8, 180);
$published_at = \Minz\Time::ago($days_ago, 'days');
// time_filter 'all' will search links until 7 days before the date
// when the user started to follow the collection
Expand Down Expand Up @@ -190,7 +190,7 @@ public function testPickDoesNotPickFromFollowedIfTooOld(): void
$now = $this->fake('dateTime');
$this->freeze($now);
/** @var int */
$days_ago = $this->fake('numberBetween', 8, 9999);
$days_ago = $this->fake('numberBetween', 8, 180);
$published_at = \Minz\Time::ago($days_ago, 'days');
$news_picker = new NewsPicker($this->user);
$link = LinkFactory::create([
Expand Down

0 comments on commit 00ac09b

Please sign in to comment.