Merge branch '346598-remove-preserve-latest-wal-locations-for-idempotent-jobs' into 'master'

Remove preserve_latest_wal_locations_for_idempotent_jobs feature flag

See merge request gitlab-org/gitlab!81024

config/feature_flags/development/preserve_latest_wal_locations_for_idempotent_jobs.yml

----
-name: preserve_latest_wal_locations_for_idempotent_jobs
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66280
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/338350
-milestone: '14.3'
-type: development
-group: group::memory
-default_enabled: true

doc/development/sidekiq/idempotent_jobs.md

@@ -190,6 +190,7 @@ that can tolerate some duplication.
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69372) in GitLab 14.3.
 > - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/338350) in GitLab 14.4.
 > - [Enabled on self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/338350) in GitLab 14.6.
+> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/346598) in GitLab 14.9. [Feature flag preserve_latest_wal_locations_for_idempotent_jobs](https://gitlab.com/gitlab-org/gitlab/-/issues/346598) removed.
 
 The deduplication always take into account the latest binary replication pointer, not the first one.
 This happens because we drop the same job scheduled for the second time and the Write-Ahead Log (WAL) is lost.
@@ -199,10 +200,3 @@ To support both deduplication and maintaining data consistency with load balanci
 we are preserving the latest WAL location for idempotent jobs in Redis.
 This way we are always comparing the latest binary replication pointer,
 making sure that we read from the replica that is fully caught up.
-
-FLAG:
-On self-managed GitLab, by default this feature is available. To hide the feature, ask an administrator to
-[disable the feature flag](../../administration/feature_flags.md) named `preserve_latest_wal_locations_for_idempotent_jobs`.
-
-This feature flag is related to GitLab development and is not intended to be used by GitLab administrators, though.
-On GitLab.com, this feature is available.

lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb

@@ -167,7 +167,6 @@ module Gitlab
         def idempotent?
           return false unless worker_klass
           return false unless worker_klass.respond_to?(:idempotent?)
-          return false unless preserve_wal_location? || !worker_klass.utilizes_load_balancing_capabilities?
 
           worker_klass.idempotent?
         end
@@ -206,8 +205,6 @@ module Gitlab
         end
 
         def job_wal_locations
-          return {} unless preserve_wal_location?
-
           job['wal_locations'] || {}
         end
 
@@ -272,10 +269,6 @@ module Gitlab
           @existing_wal_locations ||= {}
         end
 
-        def preserve_wal_location?
-          Feature.enabled?(:preserve_latest_wal_locations_for_idempotent_jobs, default_enabled: :yaml)
-        end
-
         def reschedulable?
           !scheduled? && options[:if_deduplicated] == :reschedule_once
         end

spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb

@@ -122,20 +122,6 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi
         it_behaves_like 'sets Redis keys with correct TTL'
       end
 
-      context 'when preserve_latest_wal_locations_for_idempotent_jobs feature flag is disabled' do
-        before do
-          stub_feature_flags(preserve_latest_wal_locations_for_idempotent_jobs: false)
-        end
-
-        it "does not change the existing wal locations key's TTL" do
-          expect { duplicate_job.check! }
-            .to not_change { read_idempotency_key_with_ttl(existing_wal_location_key(idempotency_key, :main)) }
-                  .from([nil, -2])
-            .and not_change { read_idempotency_key_with_ttl(existing_wal_location_key(idempotency_key, :ci)) }
-                  .from([nil, -2])
-        end
-      end
-
       it "adds the idempotency key to the jobs payload" do
         expect { duplicate_job.check! }.to change { job['idempotency_key'] }.from(nil).to(idempotency_key)
       end
@@ -186,28 +172,6 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi
       duplicate_job.check!
     end
 
-    context 'when preserve_latest_wal_locations_for_idempotent_jobs feature flag is disabled' do
-      let(:existing_wal) { {} }
-
-      before do
-        stub_feature_flags(preserve_latest_wal_locations_for_idempotent_jobs: false)
-      end
-
-      it "doesn't call Sidekiq.redis" do
-        expect(Sidekiq).not_to receive(:redis)
-
-        duplicate_job.update_latest_wal_location!
-      end
-
-      it "doesn't update a wal location to redis with an offset" do
-        expect { duplicate_job.update_latest_wal_location! }
-          .to not_change { read_range_from_redis(wal_location_key(idempotency_key, :main)) }
-                .from([])
-         .and not_change { read_range_from_redis(wal_location_key(idempotency_key, :ci)) }
-                .from([])
-      end
-    end
-
     context "when the key doesn't exists in redis" do
       let(:existing_wal) do
         {
@@ -328,20 +292,6 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi
     context 'when job is not deduplication and wal locations were not persisted' do
       it { expect(duplicate_job.latest_wal_locations).to be_empty }
     end
-
-    context 'when preserve_latest_wal_locations_for_idempotent_jobs feature flag is disabled' do
-      before do
-        stub_feature_flags(preserve_latest_wal_locations_for_idempotent_jobs: false)
-      end
-
-      it "doesn't call Sidekiq.redis" do
-        expect(Sidekiq).not_to receive(:redis)
-
-        duplicate_job.latest_wal_locations
-      end
-
-      it { expect(duplicate_job.latest_wal_locations).to eq({}) }
-    end
   end
 
   describe '#delete!' do
@@ -406,32 +356,6 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi
           let(:key) { wal_location_key(idempotency_key, :ci) }
           let(:from_value) { wal_locations[:ci] }
         end
-
-        context 'when preserve_latest_wal_locations_for_idempotent_jobs feature flag is disabled' do
-          before do
-            stub_feature_flags(preserve_latest_wal_locations_for_idempotent_jobs: false)
-          end
-
-          it_behaves_like 'does not delete key from redis', 'latest wal location keys for main database' do
-            let(:key) { existing_wal_location_key(idempotency_key, :main) }
-            let(:from_value) { wal_locations[:main] }
-          end
-
-          it_behaves_like 'does not delete key from redis', 'latest wal location keys for ci database' do
-            let(:key) { existing_wal_location_key(idempotency_key, :ci) }
-            let(:from_value) { wal_locations[:ci] }
-          end
-
-          it_behaves_like 'does not delete key from redis', 'latest wal location keys for main database' do
-            let(:key) { wal_location_key(idempotency_key, :main) }
-            let(:from_value) { wal_locations[:main] }
-          end
-
-          it_behaves_like 'does not delete key from redis', 'latest wal location keys for ci database' do
-            let(:key) { wal_location_key(idempotency_key, :ci) }
-            let(:from_value) { wal_locations[:ci] }
-          end
-        end
       end
 
       context 'when the idempotency key is not part of the job' do
@@ -666,16 +590,6 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi
       it 'returns true' do
         expect(duplicate_job).to be_idempotent
       end
-
-      context 'when preserve_latest_wal_locations_for_idempotent_jobs feature flag is disabled' do
-        before do
-          stub_feature_flags(preserve_latest_wal_locations_for_idempotent_jobs: false)
-        end
-
-        it 'returns false' do
-          expect(duplicate_job).not_to be_idempotent
-        end
-      end
     end
   end