Commit 41252358 authored by nmilojevic1's avatar nmilojevic1 Committed by Douglas Barbosa Alexandre

Remove preserve_latest_wal_locations_for_idempotent_jobs

- clean specs and documentation

other: removed preserve_latest_wal_locations_for_idempotent_jobs ff
parent df12ad25
---
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
......@@ -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.
......@@ -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
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment