Limit maximum number on related epics
In !81422 (merged) we introduced support for querying related epics of an epic. This results in N+1 queries when performing authorisation to read these epics. To mitigate this performance issue we should consider introducing a limit on the number of epics that can be related to a single epic.
The following discussion from !81422 (merged) should be addressed:
-
@egrieff started a discussion: (+5 comments) This method
#epics_readable_by_user
callsepic.readable_by?
that is defined inApplicationRecord
and checks for permissions directly:Ability.allowed?(user, "read_#{to_ability_name}".to_sym, self)
This results in N+1 queries so I tried redefining
#readable_by?
in the epic model to match the policies and improve performance (as being done for issues), but could only save extrasaml_providers
, no group hierarchy queries.here's the deifnition I tested it with
def readable_by?(user) return false unless group.licensed_feature_available?(:epics) return publicly_visible? unless user if user.can_read_all_resources? true elsif confidential? ::Group.groups_user_can(group.self_and_ancestors, user, :read_confidential_epic, same_root: true).any? elsif group.public? || (group.internal? && !user.external?) true else ::Group.groups_user_can_read_epics(group.self_and_ancestors, user, same_root: true).any? end end # Returns `true` if this Epic is publicly visible. def publicly_visible? group.public? && !confidential? end
I commented in the specs (also here) what the extra queries were, but they will vary depending on the epics group hierarchy.
-
Jan Provaznik @jprovaznik
commented:you also can reference unliminted number of epics in comments and reference redactor verifies these too - I assume the same N+1 issue is there too?
Yep, there is N+1 issue too (as expected) - similar check is done in https://gitlab.com/gitlab-org/gitlab/blob/master/lib/banzai/reference_parser/issuable_parser.rb#L12. I think the best we can do at this point is make sure we preload relevant associations (to minimize number of extra queries) and add a limit on maximimum number on related epics (out of scope of this MR).