Skip to content

Add conversation starter when using `gitlab_main_clusterwide` schema

Manoj M J [PTO - Back on 17th June] requested to merge 424990-conversation-comment into master

What does this MR do and why?

Premise

Based on the conversation in #424990 (comment 1602998078)

As we are building out cells, more and more tables are being moved from the gitlab_main schema to either gitlab_main_cell or gitlab_main_clusterwide schema. See Move some tables to gitlab_main_cell schema (!135241 - merged), as an example.

While tagging tables as gitlab_main_cell is OK, more consideration is required when tables are tagged as gitlab_main_clusterwide.

See our guidelines.

Choosing gitlab_main_clusterwide as the schema for a table could have scaling implications, so we need measures to make sure that this schema is chosen after careful consideration.

For this, we are adding a conversation note when such a change is made.

What is the change?

Whenever a MR has changes that assign the gitlab_main_clusterwide schema to any table, the following comment will be made on the diff by the bot.

Screenshot_2023-11-01_at_12.14.11_PM

This serves as an extra measure for the MR author to be sure of the change, or if they are doubtful, they could contact the grouptenant scale group.

Why is this not being made into a mandatory approval?

2 reasons:

  • File changes to db/docs/*.yml already requires a mandatory database review. Having another mandatory approval from grouptenant scale on top of this could lead to approval fatigue.
  • It is not easy to implement requiring a mandatory approval when a specific string is present in a diff. CODEOWNERS currently work only based on file path, not on the content of the diff. To make this work, we will have to invest a lot more time. The change in this MR is a good MVC, and we can always iterate on this if need be.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

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

Related to #424990 (closed)

Edited by Manoj M J [PTO - Back on 17th June]

Merge request reports