Skip to content

Refactor NamespaceOnboardingAction model to OnboardingProgress

What does this MR do?

This is a proposal to refactor the NamespaceOnboardingAction model to an OnboardingProgress model. In #294127 (closed) it was suggested to convert the namespace_onboarding_actions table to create a single record for each new namespace and use columns for separate actions, instead of creating records for each namespace/action combination, like so:

Current approach:

namepace_id action
117 1
117 2

New approach:

namepace_id action_1 action_2
117 2020-12-16 20:22:30 NULL

This has the following advantages:

  • easier queries for namespaces that finished one action but not another (instead of selecting namespace_ids where a record exists for certain actions and exclude the results from a subquery where we select the namespace_ids for which a record exist for another action, just select records where certain actions are not null and other actions are null)
  • no arbitrary monitoring window of 90 days to query for new created namespaces, instead, we just check in the table if a record for the namespace exists
  • no more need to join the namespaces table with the namespace_onboarding_actions table in order to determine the creation date of a namespace
  • less created records, instead of 1 record per namespace/action combination, we create just one record per namespace, which possibly benefits the indexes and lookup times (although the drawback is a little bit more space is required for reserving the nullable action columns)

This refactor consists of the following steps (one commit has been created for each step):

  • moved the OnboardingProgressService to the Namespaces module for clarity
  • refactored all calls to NamespaceOnboardingAction.create_action method scattered around the code to use the service as an abstraction layer for the model and make future refactors less verbose
  • added another table (since renaming tables is hard) called onboarding_progress
  • refactored the Namespaces::OnboardingProgressService service to call the new OnboardingProgress model
  • removed the old NamespaceOnboardingAction model
  • added a call to create a onboarding_progress record when creating a root namespace in the Groups::CreateService service

In a next release a migration will be addes for removing the foreign key on namespace_onboarding_actions.namespace_id and another migration to drop the table as described here. Followup issue: #296754 (closed)

Database changes

Output of database migration:

╰─ rails db:migrate:redo VERSION=20201230180202
== 20201230180202 CreateOnboardingProgress: reverting =========================
-- drop_table(:onboarding_progresses)
   -> 0.0105s
== 20201230180202 CreateOnboardingProgress: reverted (0.0161s) ================

== 20201230180202 CreateOnboardingProgress: migrating =========================
-- create_table(:onboarding_progresses)
   -> 0.0140s
== 20201230180202 CreateOnboardingProgress: migrated (0.0158s) ================

Introduced/changed queries

  • OnboardingProgress.find_by(namespace: namespace, action_column => nil)

Raw SQL:

SELECT
    "onboarding_progresses".*
FROM
    "onboarding_progresses"
WHERE
    "onboarding_progresses"."namespace_id" = 9970
    AND "onboarding_progresses"."git_write_at" IS NULL
LIMIT 1

Query Plan (from local environment): https://explain.depesz.com/s/dyUe

  • OnboardingProgress.where(namespace: namespace).where.not(action_column => nil).exists?

Raw SQL:

SELECT
    1 AS one
FROM
    "onboarding_progresses"
WHERE
    "onboarding_progresses"."namespace_id" = 9970
    AND "onboarding_progresses"."git_write_at" IS NOT NULL
LIMIT 1

Query Plan (from local environment): https://explain.depesz.com/s/5OQ8

Related to #294127 (closed)

Edited by Alex Buijs

Merge request reports