New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: optimise WH Syncs page query #4507
Conversation
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
@@ -767,7 +768,8 @@ func (u *Uploads) syncsInfo(ctx context.Context, limit, offset int, opts model.S | |||
FROM | |||
`+uploadsTableName+` | |||
WHERE | |||
1 = 1 %s | |||
1 = 1 %s ) | |||
SELECT * from filtered_uploads |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what the motivation behind splitting the query ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wasn't using the index we'd like the planner to use because of the order by id
clause
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The index which was being used previously was the id index owing to order by id
part of the query. We were unable to utilise the composite index on source_id
and also the workspace_id
. Splitting would reduce the number of records which would have to be ordered
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Limit (cost=0.56..739.94 rows=10 width=901) (actual time=165577.045..165577.764 rows=10 loops=1)
-> WindowAgg (cost=0.56..14923726.58 rows=201840 width=901) (actual time=165577.045..165577.762 rows=10 loops=1)
-> Index Scan Backward using wh_uploads_pkey on wh_uploads (cost=0.56..14920194.38 rows=201840 width=1091) (actual time=0.043..163955.218 rows=176964 loops=1)
Filter: ((source_id)::text = ANY ('{2c1sPtXLbk4cmWPC7cG0H5bFosA,2ciKRmPCIgKt5bsdv7uG6coC3Hb,2Ge8hxukiDqlfRYryRXkWrgKeoS,2GeaB8JdqfIXcM5Rvneardhw3rh,2GeaB8kEtOrLg2aq9wnQyEtgPgI,2GefDXO4QSHN3eKWxfo2V3ZKyjW,2GehjC5TzZMvIYifWy40GAquaI5,2GhzVUwijJnlL61HcXqb5niGLxo,2GjybtDGyQcN3t9Lin0uNQXb0ts,2GjybuFlrAgGu19turlbM0NT8jV,2Gjyc13u0TQx9RfD7c9JrQVfcgO,2Gjyc7PZ1CKpBEOtQ5bGfGUP9ZK,2H1axyvBuifVWlXNc85OCy0ahE0,2IEdtggGRKiM7Iz4o3UsFI09hbc,2JAS4Nvjsx1Ggos8Nz1tvykWim9,2JAS4PuabhARgGXBwLD9k8JBVKk,2JAS4SIMQPCqfm2QhFAQHxFqZsA,2JAS4TdXGhld0xYgsdualivF3RI,2OMtIMZyxTVQmNQjrfNBF4VFzLd,2OMtz840XKv86Q8spHmwlSA0ocq,2OMYPGtvfiLnj0ucJEFJ5yakTqy,2P0B6vPKnxLBYpKuKAmXuG7Sxl9,2PMDg7ENLMdUCZeg8pvjqka45Iq,2PMDNixyZpygnnhy6HVDc5Mbyuk,2PMS5FN4mVnz4runSmqSPAo7GUd,2REVTlptalDqXZuGudk6hUMOWqI,2RSiFeVYZKGg3ruXWTzfLscx4eU,2SrujLeTN36dnzW4dqfe4rsqO1W,2SrvAWAYTZfu78THXeEgVcsEWJY,2SV79l0ppqRKanqEeuKN9nfuMsd,2Ubmid4IPV9yVQ2VdMr2H3dNj9t,2UeMGbgN4OXhQAK6TcIJDdKUKUy,2VsRQgpZ7O6NnKIS6frsMguJAJw,2VsRXwwMqoGYDQipyl0XD0RF7Iu,2WpbsvLkQWZ4qvaaBK9TuIZ443Z,2YQCFf1FMcWgUXwGHiemaVKySLB,2YTgFCT13CrjTPyhMhuj87cubgz}'::text[]))
Rows Removed by Filter: 6256384
Planning Time: 0.236 ms
Execution Time: 165612.572 ms
The query plan earlier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Limit (cost=0.56..41.54 rows=10 width=902) (actual time=2477.429..2477.442 rows=10 loops=1)
-> WindowAgg (cost=0.56..757471.36 rows=185258 width=902) (actual time=2477.428..2477.440 rows=10 loops=1)
-> Index Scan using wh_uploads_source_destination_id_index on wh_uploads (cost=0.56..754229.35 rows=185258 width=1092) (actual time=0.036..2027.827 rows=177027 loops=1)
Index Cond: ((source_id)::text = ANY ('{2c1sPtXLbk4cmWPC7cG0H5bFosA,2ciKRmPCIgKt5bsdv7uG6coC3Hb,2Ge8hxukiDqlfRYryRXkWrgKeoS,2GeaB8JdqfIXcM5Rvneardhw3rh,2GeaB8kEtOrLg2aq9wnQyEtgPgI,2GefDXO4QSHN3eKWxfo2V3ZKyjW,2GehjC5TzZMvIYifWy40GAquaI5,2GhzVUwijJnlL61HcXqb5niGLxo,2GjybtDGyQcN3t9Lin0uNQXb0ts,2GjybuFlrAgGu19turlbM0NT8jV,2Gjyc13u0TQx9RfD7c9JrQVfcgO,2Gjyc7PZ1CKpBEOtQ5bGfGUP9ZK,2H1axyvBuifVWlXNc85OCy0ahE0,2IEdtggGRKiM7Iz4o3UsFI09hbc,2JAS4Nvjsx1Ggos8Nz1tvykWim9,2JAS4PuabhARgGXBwLD9k8JBVKk,2JAS4SIMQPCqfm2QhFAQHxFqZsA,2JAS4TdXGhld0xYgsdualivF3RI,2OMtIMZyxTVQmNQjrfNBF4VFzLd,2OMtz840XKv86Q8spHmwlSA0ocq,2OMYPGtvfiLnj0ucJEFJ5yakTqy,2P0B6vPKnxLBYpKuKAmXuG7Sxl9,2PMDg7ENLMdUCZeg8pvjqka45Iq,2PMDNixyZpygnnhy6HVDc5Mbyuk,2PMS5FN4mVnz4runSmqSPAo7GUd,2REVTlptalDqXZuGudk6hUMOWqI,2RSiFeVYZKGg3ruXWTzfLscx4eU,2SrujLeTN36dnzW4dqfe4rsqO1W,2SrvAWAYTZfu78THXeEgVcsEWJY,2SV79l0ppqRKanqEeuKN9nfuMsd,2Ubmid4IPV9yVQ2VdMr2H3dNj9t,2UeMGbgN4OXhQAK6TcIJDdKUKUy,2VsRQgpZ7O6NnKIS6frsMguJAJw,2VsRXwwMqoGYDQipyl0XD0RF7Iu,2WpbsvLkQWZ4qvaaBK9TuIZ443Z,2YQCFf1FMcWgUXwGHiemaVKySLB,2YTgFCT13CrjTPyhMhuj87cubgz}'::text[]))
Planning Time: 0.250 ms
Execution Time: 2516.168 ms
Query Plan now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense, have you tried order by using the composite index?
It shouldn't impact the result right?
I am a bit concern with memory/disk usage with this particular query, an CTE will be created containing all the records which might or might not fit into memory.
4bacea8
to
9dc1159
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release/1.22.x #4507 +/- ##
==================================================
+ Coverage 75.24% 75.25% +0.01%
==================================================
Files 381 381
Lines 46444 46451 +7
==================================================
+ Hits 34946 34958 +12
+ Misses 9208 9204 -4
+ Partials 2290 2289 -1 ☔ View full report in Codecov by Sentry. |
Description
Optimise WH Syncs Page Queries
Security