Skip to content

Remove trailing comma from weight system notes

Sean McGivern requested to merge migrate-weight-system-notes into master

What does this MR do?

This updates any weight system notes ending in a comma to not have one. Some had a comma because we used that to disambiguate the note from the timestamp, but then we changed the design and the comma looked weird. https://gitlab.com/gitlab-org/gitlab-ee/issues/6793 is the issue to clean up after this in 11.2.

New weight system notes are not created with a comma (since 11.0), so this is just cleaning up those created in 10.8.

Are there points in the code the reviewer needs to double check?

gitlabhq_production=> SELECT COUNT(*) FROM system_note_metadata WHERE action = 'weight';
 count
-------
 25650
(1 row)

That's the current number of rows this migration will schedule jobs for. We have about 15k rows on staging matching.

Without Redis, this migration takes well under a minute on staging:

==  ScheduleWeightSystemNoteCommaCleanup: migrating ===========================
-- transaction_open?()
   -> 0.0000s
-- execute("SET statement_timeout TO 0")
D, [2018-07-03T14:12:24.977316 #121674] DEBUG -- :    (0.9ms)  SET statement_timeout TO 0
   -> 0.0012s
-- index_exists?(:system_note_metadata, :action, {:where=>"action = 'weight'", :algorithm=>:concurrently})
   -> 0.0031s
-- add_index(:system_note_metadata, :action, {:where=>"action = 'weight'", :algorithm=>:concurrently})
D, [2018-07-03T14:12:27.254924 #121674] DEBUG -- :    (2271.2ms)  CREATE  INDEX CONCURRENTLY "index_system_note_metadata_on_action" ON "system_note_metadata"  ("action" ) WHERE action = 'weight'
   -> 2.2746s
D, [2018-07-03T14:12:27.295076 #121674] DEBUG -- :   ScheduleWeightSystemNoteCommaCleanup::Note Load (38.1ms)  SELECT  "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,')  ORDER BY "notes"."id" ASC LIMIT 1  [["system", true]]
D, [2018-07-03T14:12:27.300052 #121674] DEBUG -- :   ScheduleWeightSystemNoteCommaCleanup::Note Load (3.9ms)  SELECT  "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42958238)  ORDER BY "notes"."id" ASC LIMIT 1 OFFSET 1000  [["system", true]]
D, [2018-07-03T14:12:27.305994 #121674] DEBUG -- :    (4.1ms)  SELECT "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42958238) AND ("notes"."id" < 42959238)  [["system", true]]
-- Scheduling batch #2
D, [2018-07-03T14:12:27.311845 #121674] DEBUG -- :   ScheduleWeightSystemNoteCommaCleanup::Note Load (3.9ms)  SELECT  "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42959238)  ORDER BY "notes"."id" ASC LIMIT 1 OFFSET 1000  [["system", true]]
D, [2018-07-03T14:12:27.317117 #121674] DEBUG -- :    (4.1ms)  SELECT "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42959238) AND ("notes"."id" < 42960238)  [["system", true]]
-- Scheduling batch #3
D, [2018-07-03T14:12:27.326021 #121674] DEBUG -- :   ScheduleWeightSystemNoteCommaCleanup::Note Load (3.9ms)  SELECT  "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42960238)  ORDER BY "notes"."id" ASC LIMIT 1 OFFSET 1000  [["system", true]]
D, [2018-07-03T14:12:27.331156 #121674] DEBUG -- :    (4.1ms)  SELECT "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42960238) AND ("notes"."id" < 42961241)  [["system", true]]
-- Scheduling batch #4
D, [2018-07-03T14:12:27.338037 #121674] DEBUG -- :   ScheduleWeightSystemNoteCommaCleanup::Note Load (3.7ms)  SELECT  "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42961241)  ORDER BY "notes"."id" ASC LIMIT 1 OFFSET 1000  [["system", true]]
D, [2018-07-03T14:12:27.342969 #121674] DEBUG -- :    (4.0ms)  SELECT "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42961241) AND ("notes"."id" < 42962241)  [["system", true]]
-- Scheduling batch #5
D, [2018-07-03T14:12:27.349026 #121674] DEBUG -- :   ScheduleWeightSystemNoteCommaCleanup::Note Load (3.7ms)  SELECT  "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42962241)  ORDER BY "notes"."id" ASC LIMIT 1 OFFSET 1000  [["system", true]]
D, [2018-07-03T14:12:27.354018 #121674] DEBUG -- :    (3.8ms)  SELECT "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42962241) AND ("notes"."id" < 42963241)  [["system", true]]
-- Scheduling batch #6
D, [2018-07-03T14:12:27.360071 #121674] DEBUG -- :   ScheduleWeightSystemNoteCommaCleanup::Note Load (3.8ms)  SELECT  "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42963241)  ORDER BY "notes"."id" ASC LIMIT 1 OFFSET 1000  [["system", true]]
D, [2018-07-03T14:12:27.364828 #121674] DEBUG -- :    (3.8ms)  SELECT "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42963241) AND ("notes"."id" < 42964241)  [["system", true]]
-- Scheduling batch #7
D, [2018-07-03T14:12:27.371195 #121674] DEBUG -- :   ScheduleWeightSystemNoteCommaCleanup::Note Load (3.8ms)  SELECT  "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42964241)  ORDER BY "notes"."id" ASC LIMIT 1 OFFSET 1000  [["system", true]]
D, [2018-07-03T14:12:27.378053 #121674] DEBUG -- :    (4.1ms)  SELECT "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42964241) AND ("notes"."id" < 42965241)  [["system", true]]
-- Scheduling batch #8
D, [2018-07-03T14:12:27.384094 #121674] DEBUG -- :   ScheduleWeightSystemNoteCommaCleanup::Note Load (3.7ms)  SELECT  "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42965241)  ORDER BY "notes"."id" ASC LIMIT 1 OFFSET 1000  [["system", true]]
D, [2018-07-03T14:12:27.388662 #121674] DEBUG -- :    (3.7ms)  SELECT "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42965241) AND ("notes"."id" < 42966241)  [["system", true]]
-- Scheduling batch #9
D, [2018-07-03T14:12:27.395613 #121674] DEBUG -- :   ScheduleWeightSystemNoteCommaCleanup::Note Load (4.9ms)  SELECT  "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42966241)  ORDER BY "notes"."id" ASC LIMIT 1 OFFSET 1000  [["system", true]]
D, [2018-07-03T14:12:27.400291 #121674] DEBUG -- :    (3.7ms)  SELECT "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42966241) AND ("notes"."id" < 42967241)  [["system", true]]
-- Scheduling batch #10
D, [2018-07-03T14:12:27.405752 #121674] DEBUG -- :   ScheduleWeightSystemNoteCommaCleanup::Note Load (3.5ms)  SELECT  "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42967241)  ORDER BY "notes"."id" ASC LIMIT 1 OFFSET 1000  [["system", true]]
D, [2018-07-03T14:12:27.410653 #121674] DEBUG -- :    (4.0ms)  SELECT "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42967241) AND ("notes"."id" < 42968241)  [["system", true]]
-- Scheduling batch #11
D, [2018-07-03T14:12:27.414413 #121674] DEBUG -- :   ScheduleWeightSystemNoteCommaCleanup::Note Load (1.7ms)  SELECT  "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42968241)  ORDER BY "notes"."id" ASC LIMIT 1 OFFSET 1000  [["system", true]]
D, [2018-07-03T14:12:27.417775 #121674] DEBUG -- :    (1.8ms)  SELECT "notes"."id" FROM "notes" INNER JOIN system_note_metadata ON notes.id = system_note_metadata.note_id WHERE "notes"."system" = 't' AND "system_note_metadata"."action" = 'weight' AND (notes.note LIKE '%,') AND ("notes"."id" >= 42968241)  [["system", true]]
-- Scheduling batch #12
-- transaction_open?()
   -> 0.0000s
-- select_one("SELECT current_setting('server_version_num') AS v")
D, [2018-07-03T14:12:27.419356 #121674] DEBUG -- :    (1.0ms)  SELECT current_setting('server_version_num') AS v
   -> 0.0014s
-- execute("SET statement_timeout TO 0")
D, [2018-07-03T14:12:27.420867 #121674] DEBUG -- :    (1.0ms)  SET statement_timeout TO 0
   -> 0.0014s
-- index_exists?(:system_note_metadata, :action, {:where=>"action = 'weight'", :algorithm=>:concurrently})
   -> 0.0048s
-- remove_index(:system_note_metadata, {:where=>"action = 'weight'", :algorithm=>:concurrently, :column=>:action})
D, [2018-07-03T14:12:27.445396 #121674] DEBUG -- :    (17.5ms)  DROP INDEX "index_system_note_metadata_on_action"
   -> 0.0195s
==  ScheduleWeightSystemNoteCommaCleanup: migrated (2.4695s) ==================```

The updates performed by the background migration look like this:

UPDATE "notes"
SET "note" = TRIM(TRAILING ',' FROM note), "note_html" = NULL
WHERE "notes"."id" IN (...)

Why was this MR needed?

It's better to clean up this data so we don't need to support the workarounds forever.

Does this MR meet the acceptance criteria?

  • Changelog entry added, if necessary
  • Tests added for this feature/bug
  • Conform by the code review guidelines
    • Has been reviewed by a Backend maintainer
    • Has been reviewed by a Database specialist
  • EE specific content should be in the top level /ee folder

What are the relevant issue numbers?

Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/6347.

Edited by Yorick Peterse

Merge request reports