-
Notifications
You must be signed in to change notification settings - Fork 429
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
feat: re-add totals and limits #2631
Conversation
Add broken test to prove issue when not using distinct
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Uffizzi Preview |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2631 +/- ##
=======================================
Coverage 95.44% 95.44%
=======================================
Files 976 976
Lines 27319 27327 +8
=======================================
+ Hits 26074 26082 +8
Misses 1245 1245
☔ View full report in Codecov by Sentry. |
return Environment.objects.all() | ||
queryset = Environment.objects.all() | ||
|
||
if self.action == "retrieve": |
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.
Can we add the SQL generated by this and the results of the explain analyze (performed on the production database).
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=45.19..45.21 rows=1 width=724) (actual time=0.089..0.090 rows=1 loops=1)
-> GroupAggregate (cost=45.19..45.21 rows=1 width=724) (actual time=0.088..0.089 rows=1 loops=1)
Group Key: environments_environment.id, projects_project.id, organisations_organisation.id
-> Sort (cost=45.19..45.19 rows=1 width=720) (actual time=0.084..0.085 rows=1 loops=1)
Sort Key: environments_environment.id, projects_project.id, organisations_organisation.id
Sort Method: quicksort Memory: 25kB
-> Nested Loop (cost=1.15..45.18 rows=1 width=720) (actual time=0.078..0.080 rows=1 loops=1)
-> Nested Loop (cost=0.86..44.81 rows=1 width=637) (actual time=0.064..0.065 rows=1 loops=1)
-> Nested Loop Left Join (cost=0.57..36.50 rows=1 width=555) (actual time=0.049..0.051 rows=1 loops=1)
-> Index Scan using environments_environment_api_key_b1cbb73a_like on environments_environment (cost=0.29..8.30 rows=1 width=551) (actual time=0.024..0.024 rows=1 loops=1)
Index Cond: ((api_key)::text = 'c9qkLTj6BAgmxpUyPoZXYi'::text)
Filter: (deleted_at IS NULL)
-> Index Scan using features_featuresegment_environment_id_05e7007b on features_featuresegment (cost=0.29..28.11 rows=9 width=8) (actual time=0.023..0.024 rows=1 loops=1)
Index Cond: (environments_environment.id = environment_id)
-> Index Scan using projects_project_pkey on projects_project (cost=0.29..8.30 rows=1 width=82) (actual time=0.013..0.014 rows=1 loops=1)
Index Cond: (id = environments_environment.project_id)
-> Index Scan using organisations_organisation_pkey on organisations_organisation (cost=0.29..0.37 rows=1 width=83) (actual time=0.013..0.013 rows=1 loops=1)
Index Cond: (id = projects_project.organisation_id)
Planning Time: 0.998 ms
Execution Time: 0.187 ms
This one should be ok as we're not having to add any distinct
keywords.
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.
Sorry, here is the sql too:
select
"environments_environment"."id",
"environments_environment"."deleted_at",
"environments_environment"."name",
"environments_environment"."created_date",
"environments_environment"."description",
"environments_environment"."project_id",
"environments_environment"."api_key",
"environments_environment"."minimum_change_request_approvals",
"environments_environment"."webhooks_enabled",
"environments_environment"."webhook_url",
"environments_environment"."allow_client_traits",
"environments_environment"."updated_at",
"environments_environment"."banner_text",
"environments_environment"."banner_colour",
"environments_environment"."hide_disabled_flags",
"environments_environment"."use_mv_v2_evaluation",
"environments_environment"."hide_sensitive_data",
COUNT("features_featuresegment"."id") as "total_segment_overrides",
"projects_project"."id",
"projects_project"."deleted_at",
"projects_project"."uuid",
"projects_project"."name",
"projects_project"."created_date",
"projects_project"."organisation_id",
"projects_project"."hide_disabled_flags",
"projects_project"."enable_dynamo_db",
"projects_project"."prevent_flag_defaults",
"projects_project"."enable_realtime_updates",
"projects_project"."only_allow_lower_case_feature_names",
"projects_project"."feature_name_regex",
"projects_project"."max_segments_allowed",
"projects_project"."max_features_allowed",
"projects_project"."max_segment_overrides_allowed",
"organisations_organisation"."id",
"organisations_organisation"."deleted_at",
"organisations_organisation"."uuid",
"organisations_organisation"."name",
"organisations_organisation"."has_requested_features",
"organisations_organisation"."webhook_notification_email",
"organisations_organisation"."created_date",
"organisations_organisation"."alerted_over_plan_limit",
"organisations_organisation"."stop_serving_flags",
"organisations_organisation"."restrict_project_create_to_admin",
"organisations_organisation"."persist_trait_data",
"organisations_organisation"."block_access_to_admin",
"organisations_organisation"."feature_analytics"
from
"environments_environment"
left outer join "features_featuresegment" on
("environments_environment"."id" = "features_featuresegment"."environment_id")
inner join "projects_project" on
("environments_environment"."project_id" = "projects_project"."id")
inner join "organisations_organisation" on
("projects_project"."organisation_id" = "organisations_organisation"."id")
where
("environments_environment"."deleted_at" is null
and "environments_environment"."api_key" = '<key>')
group by
"environments_environment"."id",
"projects_project"."id",
"organisations_organisation"."id"
limit 21
Changes
Re-add totals and limits removed in #2625 due to performance issues.
Uses separate queries instead of
annotate(Count(..., distinct=True))
to avoid further performance issues.How did you test this code?
Updated the test from the previous iteration to ensure its verifying that the numbers are correct. I've been unable to really reproduce the performance issues we saw in production, but there is a lot less black magic going on with this approach. The additional queries should have a negligible impact on performance.