Commit d651d3d6 authored by Stan Hu's avatar Stan Hu Committed by Adam Hegyi

Enable DISTINCT optimization for ObjectHierarchy globally

This uses the feature flag and work in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56509 to ensure
the query planner chooses the nested join instead of the more expensive
hashed join. When the `use_distinct_for_all_object_hierarchy` feature
flag is used globally, it will apply to all queries, not just the ones
that have manually enabled them via the `options` hash while creating
`ObjectHierarchy`.

This change works around a PostgreSQL query planner bug that is detailed
in https://gitlab.com/gitlab-com/gl-infra/production/-/issues/3894.

Related issues:

1. https://gitlab.com/gitlab-com/gl-infra/production/-/issues/4024
2. https://gitlab.com/groups/gitlab-org/-/epics/5617
parent ea1ddba5
---
title: Enable DISTINCT optimization for ObjectHierarchy globally
merge_request: 57047
author:
type: changed
---
name: use_distinct_for_all_object_hierarchy
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57047
rollout_issue_url:
milestone: '13.11'
type: development
group: group::database
default_enabled: false
......@@ -96,7 +96,7 @@ module Gitlab
base_cte = base_and_descendants_cte(with_depth: true).apply_to(model.all).distinct
if with_depth
read_only(model.from(Arel::Nodes::As.new(recursive_query.arel, objects_table)).order(depth: :asc))
read_only(model.from(Arel::Nodes::As.new(base_cte.arel, objects_table)).order(depth: :asc))
else
read_only(remove_depth_and_maintain_order(base_cte, hierarchy_order: :asc))
end
......@@ -161,7 +161,12 @@ module Gitlab
# Use distinct on the Namespace queries to avoid bad planner behavior in PG11.
def use_distinct?
(model <= Namespace) && options[:use_distinct]
return unless model <= Namespace
# Global use_distinct_for_all_object_hierarchy takes precedence over use_distinct_in_object_hierarchy
return true if Feature.enabled?(:use_distinct_for_all_object_hierarchy)
return options[:use_distinct] if options.key?(:use_distinct)
false
end
# Remove the extra `depth` field using an INNER JOIN to avoid breaking UNION queries
......
......@@ -187,6 +187,21 @@ RSpec.describe Gitlab::ObjectHierarchy do
context 'when the use_distinct_in_object_hierarchy feature flag is enabled' do
before do
stub_feature_flags(use_distinct_in_object_hierarchy: true)
stub_feature_flags(use_distinct_for_all_object_hierarchy: false)
end
it_behaves_like 'Gitlab::ObjectHierarchy test cases'
it 'calls DISTINCT' do
expect(parent.self_and_descendants.to_sql).to include("DISTINCT")
expect(child2.self_and_ancestors.to_sql).to include("DISTINCT")
end
end
context 'when the use_distinct_for_all_object_hierarchy feature flag is enabled' do
before do
stub_feature_flags(use_distinct_in_object_hierarchy: false)
stub_feature_flags(use_distinct_for_all_object_hierarchy: true)
end
it_behaves_like 'Gitlab::ObjectHierarchy test cases'
......@@ -200,6 +215,7 @@ RSpec.describe Gitlab::ObjectHierarchy do
context 'when the use_distinct_in_object_hierarchy feature flag is disabled' do
before do
stub_feature_flags(use_distinct_in_object_hierarchy: false)
stub_feature_flags(use_distinct_for_all_object_hierarchy: false)
end
it_behaves_like 'Gitlab::ObjectHierarchy test cases'
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment