Skip to content

WIP: Change db/schema.rb merge policy to --theirs

Toon Claes requested to merge tc-remove-merge-db-schema into master

Normally, all schema changes should go into gitlab-ce, so consider the schema of gitlab-ce to be the SSOT.

See https://gitlab.com/gitlab-org/gitlab-ee/issues/12434

Reasoning

Consider 3 types of feature changes

  • Type A feature: Feature built with only a gitlab-ce merge request, and no gitlab-ee MR.
  • Type B feature: Feature built with a gitlab-ce merge request and a gitlab-ee merge request.
  • Type C feature: Feature built with only a gitlab-ee merge request, and no gitlab-ce MR.

Possible types of conflicts in db/schema.rb

Scenario

Feature C MR can never create a conflict, cause if it would touch schema.rb it should have a gitlab-ce MR also.

Scenario

Feature A MR cannot create a conflict with another A MR, cause the conflict should be resolved before one or the other can get merged into gitlab-ce master.

Scenario

A conflict can show up whenever a B feature has a diff between the schema.rb in the gitlab-ce MR and gitlab-ee MR. The conflict will happen after the B gitlab-ee MR is merged and the changes from the B gitlab-ce MR end up in gitlab-ee by the merge-train. In this case, previously, git merge strategy --ours was used, preferring the changes from gitlab-ee. After that, all following gitlab-ce MRs (either A or B) that end up in gitlab-ee through the merge train, might result in schema changes from gitlab-ce being discarded.

Proposal

That's why I'm proposing to use git merge strategy --theirs. This will threat gitlab-ce as the SSOT and might overwrite schema changes done in a B gitlab-ee MR. Is this a problem? Normally it shouldn't. The B gitlab-ce MR should also have the correct schema for the feature

Scenario

A conflict can also show up when a A gitlab-ce MR is merged between a B gitlab-ee MR and the matching B gitlab-ce MR. (Assuming the B MRs are merged in correct order: gitlab-ee before gitlab-ce)

Proposal

With the proposal of using --theirs the A gitlab-ce MR would wipe out the changes done by the B gitlab-ee MR. This is unfortunate1, but... When the A gitlab-ce MR was merged, it should make sure the B gitlab-ce MR changes the schema without conflict. This means the A gitlab-ce MR merge action might make it impossible to merge the B gitlab-ce MR (which was probably MWPS). This should sound the alarm bell of the maintainer setting MWPS, or the original author of the MR. And someone should resolve the conflict manually. Or, probably in most cases, the tooling around merging the db/schema.rb should resolve the conflict in gitlab-ce automatically, at the moment B gitlab-ce MR is merged.

  1. During the period when the A gitlab-ce MR wiped the schema, it might happen the gitlab-ee master pipelines might fail. It's painful, but on the other hand it might be a good early indication something is wrong. If we raise awareness around this schema merge policy I think we can tackle these schema conflicts quickly after happening. ↩

Edited by Toon Claes

Merge request reports