Skip to content
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

Recents API Improvements #42660

Merged
merged 18 commits into from
May 16, 2024
Merged

Recents API Improvements #42660

merged 18 commits into from
May 16, 2024

Conversation

escherize
Copy link
Contributor

@escherize escherize commented May 14, 2024

Resolves #41957
Fixes #42665

This addresses an opportunity @dpsutton mentioned about improving endpoint's performance.

Main entrypoint is recent-views/get-list.

So, instead of querying for each model one at a time, we get all of the entity info up front for each recent_view in a query for its type. e.g. all question and dataset ids are selected from :model/Card, joined with collection and moderation_review.

This data is indexed by model and id, and post-process uses that data structure to look up relevant values to return the Item shape.

  • filter all things through mi/can-read?
  • return effective-parent for collection's parent collection

@escherize escherize requested a review from dpsutton May 14, 2024 20:08
@escherize escherize requested a review from camsaul as a code owner May 14, 2024 20:09
@metabase-bot metabase-bot bot added the .Team/AdminWebapp Admin and Webapp team label May 14, 2024
@escherize escherize added the no-backport Do not backport this PR to any branch label May 14, 2024
@escherize escherize requested a review from a team May 14, 2024 20:20
Copy link

replay-io bot commented May 14, 2024

Status In Progress ↗︎ 51 / 52
Commit a048fc3
Results
⚠️ 1 Flaky
2508 Passed

src/metabase/models/recent_views.clj Outdated Show resolved Hide resolved
src/metabase/models/recent_views.clj Outdated Show resolved Hide resolved
src/metabase/models/recent_views.clj Outdated Show resolved Hide resolved
src/metabase/models/recent_views.clj Outdated Show resolved Hide resolved
src/metabase/models/recent_views.clj Show resolved Hide resolved
src/metabase/models/recent_views.clj Outdated Show resolved Hide resolved
src/metabase/models/recent_views.clj Outdated Show resolved Hide resolved
src/metabase/models/recent_views.clj Outdated Show resolved Hide resolved
@escherize escherize marked this pull request as draft May 16, 2024 16:18
@escherize
Copy link
Contributor Author

I'm converting this to a draft until I include the improved parent_collection support

@dpsutton dpsutton added this to the 0.50 milestone May 16, 2024
- always return effective parent collections
@escherize escherize requested a review from noahmoss May 16, 2024 21:18
@escherize escherize changed the title Recents API perf improvement Recents API Improvements May 16, 2024
@escherize escherize marked this pull request as ready for review May 16, 2024 21:19
@escherize escherize requested review from noahmoss and a team May 16, 2024 21:40
@escherize escherize enabled auto-merge (squash) May 16, 2024 22:14
@escherize escherize merged commit 7ab065f into master May 16, 2024
107 checks passed
@escherize escherize deleted the bcm-recents-api-perf-improvement branch May 16, 2024 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-backport Do not backport this PR to any branch .Team/AdminWebapp Admin and Webapp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reduce number of queries. We have an N+1 situation Beef up Recents 🥩
3 participants