BG migration to fix incorrect job artifacts expire_at on self-managed

What does this MR do and why?

Fix remaining incorrect expire_at on self-managed instances that was set by !47723 (merged). This gives instances that may not have fully executed the fix in DestroyBatchService to still have a way to remove the incorrect values.

This MR adds a temporary index, which will be removed later on, tracked in #368979 (closed).

Screenshots or screen recordings

These are strongly recommended to assist reviewers and reduce the time to merge your change.

DB Queries

Create index:

CREATE INDEX tmp_index_ci_job_artifacts_on_id_expire_at_file_type_trace ON ci_job_artifacts USING btree (id, expire_at, file_type) WHERE (((date_part('day'::text, timezone('UTC'::text, expire_at)) = ANY (ARRAY[(21)::double precision, (22)::double precision, (23)::double precision])) AND (date_part('minute'::text, timezone('UTC'::text, expire_at)) = ANY (ARRAY[(0)::double precision, (30)::double precision, (45)::double precision])) AND (date_part('second'::text, timezone('UTC'::text, expire_at)) = (0)::double precision)) OR (file_type = 3));

Batching and update:

SELECT
    "ci_job_artifacts"."id"
FROM
    "ci_job_artifacts"
WHERE
    "ci_job_artifacts"."id" BETWEEN 1 AND 100
ORDER BY
    "ci_job_artifacts"."id" ASC
LIMIT 1

--

SELECT
    "ci_job_artifacts"."id"
FROM
    "ci_job_artifacts"
WHERE
    "ci_job_artifacts"."id" BETWEEN 1 AND 100
    AND "ci_job_artifacts"."id" >= 1
ORDER BY
    "ci_job_artifacts"."id" ASC
LIMIT 1 OFFSET 10

--

UPDATE
    "ci_job_artifacts"
SET
    "expire_at" = NULL
WHERE
    "ci_job_artifacts"."id" BETWEEN 1 AND 100
    AND "ci_job_artifacts"."id" >= 1
    AND "ci_job_artifacts"."id" < 11
    AND (EXTRACT(day FROM timezone('UTC', expire_at)) IN (21, 22, 23)
        AND EXTRACT(minute FROM timezone('UTC', expire_at)) IN (0, 30, 45)
        AND EXTRACT(second FROM timezone('UTC', expire_at)) = 0
        OR "ci_job_artifacts"."file_type" = 3)

--

SELECT
    "ci_job_artifacts"."id"
FROM
    "ci_job_artifacts"
WHERE
    "ci_job_artifacts"."id" BETWEEN 1 AND 100
    AND "ci_job_artifacts"."id" >= 11
ORDER BY
    "ci_job_artifacts"."id" ASC
LIMIT 1 OFFSET 10

--

UPDATE
    "ci_job_artifacts"
SET
    "expire_at" = NULL
WHERE
    "ci_job_artifacts"."id" BETWEEN 1 AND 100
    AND "ci_job_artifacts"."id" >= 11
    AND (EXTRACT(day FROM timezone('UTC', expire_at)) IN (21, 22, 23)
        AND EXTRACT(minute FROM timezone('UTC', expire_at)) IN (0, 30, 45)
        AND EXTRACT(second FROM timezone('UTC', expire_at)) = 0
        OR "ci_job_artifacts"."file_type" = 3)

query plans:

• batching: https://console.postgres.ai/gitlab/gitlab-production-ci/sessions/11075/commands/40091
• update: https://postgres.ai/console/gitlab/gitlab-production-ci/sessions/11075/commands/40095

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.

• I have evaluated the MR acceptance checklist for this MR.

Related to #355833 (closed)

lib/gitlab/background_migration/remove_backfilled_job_artifacts_expire_at.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # This detects and fixes job artifacts that have `expire_at` wrongly backfilled by the migration
+    # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47723.
+    # These job artifacts will not be deleted and will have their `expire_at` removed.
+    class RemoveBackfilledJobArtifactsExpireAt < BatchedMigrationJob
+      # The migration would have backfilled `expire_at`
+      # to midnight on the 22nd of the month of the local timezone,
+      # storing it as UTC time in the database.
+      #
+      # If the timezone setting has changed since the migration,
+      # the `expire_at` stored in the database could have changed to a different local time other than midnight.
+      # For example:
+      # - changing timezone from UTC+02:00 to UTC+02:30 would change the `expire_at` in local time 00:00:00 to 00:30:00.
+      # - changing timezone from UTC+00:00 to UTC-01:00 would change the `expire_at` in local time 00:00:00 to 23:00:00
+      #   on the previous day (21st).
+      #
+      # Therefore job artifacts that have `expire_at` exactly on the 00, 30 or 45 minute mark
+      # on the dates 21, 22, 23 of the month will not be deleted.
+      # https://en.wikipedia.org/wiki/List_of_UTC_time_offsets
+      EXPIRES_ON_21_22_23_AT_MIDNIGHT_IN_TIMEZONE = <<~SQL
+        EXTRACT(day FROM timezone('UTC', expire_at)) IN (21, 22, 23)
+        AND EXTRACT(minute FROM timezone('UTC', expire_at)) IN (0, 30, 45)
+        AND EXTRACT(second FROM timezone('UTC', expire_at)) = 0
+      SQL
+
+      def perform
+        each_sub_batch(
+          operation_name: :update_all
+        ) do |sub_batch|
+          sub_batch.where(EXPIRES_ON_21_22_23_AT_MIDNIGHT_IN_TIMEZONE)
+                   .or(sub_batch.where(file_type: 3))
+                   .update_all(expire_at: nil)
+        end
+      end
+    end
+  end
+end

[Krasimir Angelov]

@alberts-gitlab @ahegyi When we use apply_additional_filters, the same filters should be applied here as batching_scope, otherwise the batch generated by the job class will be different (may include more rows) from the batch generated by the batching strategy.

Yes, it's confusing and error-prone, this is one of the reasons for switching to scope_to - the filters defined this way will be used by both the batching strategy and the job class, and we do not have to define custom batching strategy class.

We're still fine here, as we apply the filters on the yielded relation (sub_batch). I'll update this as part of !96478 (merged) anyway,

[Krasimir Angelov]

Updated with !96478 (2097adef), can you please have a look and confirm all is good?

[Adam Hegyi]

Thanks, @krasio! I'll take a look.