Skip to content

Introduce linear descendants feature flag

Francisco Javier López (ex-Gitlab) requested to merge fj-introduce-linear-descendants-feature-flag into master

What does this MR do and why?

In https://gitlab.com/gitlab-org/gitlab/-/issues/342871#note_809803014, it was discovered and raised that the self_and_descendants method was returning not enough records in certain circumstances when the feature flag traversal_ids_btree.

So what we need now is to, somehow disable that behavior, and start returning valid data while we fix it.

The problem we have is that the traversal_ids_btree has been enabled for quite some time and we have merged other related code based on the times methods using that feature flag provide. Therefore, just turning it off, could bring some other performance problems to the table.

The other approach would be to turn off the global use_traversal_ids, but this would be like hunting flies with a hammer. There are other methods using this feature flag and if we turn it off just to fix self_and_descendants, it would also roll back the other methods to the recursive approach.

Therefore, we need to just short-circuit the self_and_descendants method.

The premises are the following:

  • The use_traversal_ids ff has been enabled in production for quite some time. It's disabled in the yaml file, so only .com is using the linear approach.
  • The traversal_ids_btree ff has been enabled in production for quite some time. It's disabled in the yaml file, so only .com is using the linear approach.

The idea with this MR is to introduce a new ff use_traversal_ids_for_descendants_scopes . This feature flag will be enabled by default, that way, when it's deployed, the behavior in production will be the same. At the same time, the overall logic will be also based on use_traversal_ids. This way, self-hosted instances will use the recursive approach even if the new ff is enabled by default in the yaml file.

Then, once the MR is deployed, we can turn the ff off for use un prod and use the recursive approach just for this method while the other ones are still using the linear version.

How to set up and validate locally

  • Disable use_traversal_ids_for_descendants_scopes
  • Run in the Rails console:
Group.where(id: Group.first).self_and_descendants.explain

It will show we're using a recursive database query.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Francisco Javier López (ex-Gitlab)

Merge request reports