From 00ac09b43d786efd47c0552d1cb3eafac4116a22 Mon Sep 17 00:00:00 2001 From: Marien Fressinaud Date: Wed, 2 Oct 2024 17:47:22 +0200 Subject: [PATCH] tec: Improve the performance of news refreshing --- ...2AddCreatedAtIndexOnLinksToCollections.php | 28 +++++++++++ src/models/dao/links/NewsQueries.php | 46 +++++++++++++++++-- src/schema.sql | 1 + src/services/NewsPicker.php | 21 +-------- tests/services/NewsPickerTest.php | 4 +- 5 files changed, 74 insertions(+), 26 deletions(-) create mode 100644 src/migrations/Migration202410020002AddCreatedAtIndexOnLinksToCollections.php diff --git a/src/migrations/Migration202410020002AddCreatedAtIndexOnLinksToCollections.php b/src/migrations/Migration202410020002AddCreatedAtIndexOnLinksToCollections.php new file mode 100644 index 00000000..dffbc6ba --- /dev/null +++ b/src/migrations/Migration202410020002AddCreatedAtIndexOnLinksToCollections.php @@ -0,0 +1,28 @@ +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; + } +} diff --git a/src/models/dao/links/NewsQueries.php b/src/models/dao/links/NewsQueries.php index e3287f7f..5ad7be32 100644 --- a/src/models/dao/links/NewsQueries.php +++ b/src/models/dao/links/NewsQueries.php @@ -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 = <<= :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; } /** diff --git a/src/schema.sql b/src/schema.sql index f6a1b965..7aab2567 100644 --- a/src/schema.sql +++ b/src/schema.sql @@ -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, diff --git a/src/services/NewsPicker.php b/src/services/NewsPicker.php index be2a4316..1dd7daf3 100644 --- a/src/services/NewsPicker.php +++ b/src/services/NewsPicker.php @@ -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); } } diff --git a/tests/services/NewsPickerTest.php b/tests/services/NewsPickerTest.php index dda784b2..f656a8c2 100644 --- a/tests/services/NewsPickerTest.php +++ b/tests/services/NewsPickerTest.php @@ -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 @@ -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([