Skip to content

Commit c1f257c

Browse files
kathapkathap
committed
Drop live-pollable guard from delete-retry jobs, only needed for update case
1 parent abe8c6b commit c1f257c

4 files changed

Lines changed: 0 additions & 118 deletions

‎app/jobs/runtime/service_operations_binding_delete_stuck_in_progress_retry.rb‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ def retry_stuck(operation_model, instance_model, foreign_key, jobs_operation)
3737
where(Sequel[:jobs][:state] => [PollableJobModel::POLLING_STATE, PollableJobModel::FAILED_STATE]).
3838
where(Sequel[:jobs][:operation] => jobs_operation).
3939
exclude(Sequel[:delayed_jobs][:failed_at] => nil).
40-
exclude(live_pollable_exists(operation_model, instance_table, jobs_operation)).
4140
select(
4241
Sequel[:jobs][:guid].as(:pollable_guid),
4342
Sequel[operation_table][:id].as(:op_id),
@@ -97,25 +96,6 @@ def default_maximum_duration_seconds
9796
Config.config.get(:broker_client_max_async_poll_duration_minutes).minutes
9897
end
9998

100-
# NOT EXISTS guard: skip a binding if it still has a pollable job actively driving
101-
# THIS operation — state POLLING or PROCESSING AND backed by a delayed_job that has
102-
# NOT permanently failed (failed_at IS NULL, or no delayed_job row yet). A stale,
103-
# permanently-failed pollable left behind by a previous operation on the same
104-
# binding must NOT trigger a spurious re-enqueue. A POLLING pollable whose
105-
# delayed_job IS failed is itself stuck (the DB flip happened before the failure
106-
# hook could write FAILED) and must NOT count as live. Correlated
107-
# (resource_guid = binding.guid) so a NULL jobs.resource_guid elsewhere cannot
108-
# poison the result the way a NOT IN subquery would.
109-
def live_pollable_exists(operation_model, instance_table, jobs_operation)
110-
operation_model.db[:jobs].
111-
left_join(:delayed_jobs, guid: Sequel[:jobs][:delayed_job_guid]).
112-
where(Sequel[:jobs][:operation] => jobs_operation).
113-
where(Sequel[:jobs][:state] => [PollableJobModel::POLLING_STATE, PollableJobModel::PROCESSING_STATE]).
114-
where(Sequel[:delayed_jobs][:failed_at] => nil).
115-
where(Sequel[:jobs][:resource_guid] => Sequel[instance_table][:guid]).
116-
exists
117-
end
118-
11999
def logger
120100
@logger ||= Steno.logger('cc.background.service-operations-binding-delete-stuck-in-progress-retry')
121101
end

‎app/jobs/runtime/service_operations_delete_stuck_in_progress_retry.rb‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ def retry_stuck(operation_model, instance_model, foreign_key, jobs_operation)
3636
where(Sequel[:jobs][:state] => [PollableJobModel::POLLING_STATE, PollableJobModel::FAILED_STATE]).
3737
where(Sequel[:jobs][:operation] => jobs_operation).
3838
exclude(Sequel[:delayed_jobs][:failed_at] => nil).
39-
exclude(live_pollable_exists(operation_model, instance_table, jobs_operation)).
4039
select(
4140
Sequel[:jobs][:guid].as(:pollable_guid),
4241
Sequel[operation_table][:id].as(:op_id),
@@ -96,25 +95,6 @@ def default_maximum_duration_seconds
9695
Config.config.get(:broker_client_max_async_poll_duration_minutes).minutes
9796
end
9897

99-
# NOT EXISTS guard: skip a resource if it still has a pollable job actively driving
100-
# THIS operation — state POLLING or PROCESSING AND backed by a delayed_job that has
101-
# NOT permanently failed (failed_at IS NULL, or no delayed_job row yet). A stale,
102-
# permanently-failed pollable left behind by a previous operation on the same
103-
# resource must NOT trigger a spurious re-enqueue. A POLLING pollable whose
104-
# delayed_job IS failed is itself stuck (the DB flip happened before the failure
105-
# hook could write FAILED) and must NOT count as live. Correlated
106-
# (resource_guid = instance.guid) so a NULL jobs.resource_guid elsewhere cannot
107-
# poison the result the way a NOT IN subquery would.
108-
def live_pollable_exists(operation_model, instance_table, jobs_operation)
109-
operation_model.db[:jobs].
110-
left_join(:delayed_jobs, guid: Sequel[:jobs][:delayed_job_guid]).
111-
where(Sequel[:jobs][:operation] => jobs_operation).
112-
where(Sequel[:jobs][:state] => [PollableJobModel::POLLING_STATE, PollableJobModel::PROCESSING_STATE]).
113-
where(Sequel[:delayed_jobs][:failed_at] => nil).
114-
where(Sequel[:jobs][:resource_guid] => Sequel[instance_table][:guid]).
115-
exists
116-
end
117-
11898
def logger
11999
@logger ||= Steno.logger('cc.background.service-operations-delete-stuck-in-progress-retry')
120100
end

‎spec/unit/jobs/runtime/service_operations_binding_delete_stuck_in_progress_retry_spec.rb‎

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -49,18 +49,6 @@ def prepare_stuck_binding(
4949
{ binding: binding, pjob: pjob, delayed_job: dj }
5050
end
5151

52-
# Attach an additional live pollable job (POLLING/PROCESSING, delayed_job NOT failed)
53-
# for the same binding + operation. Mirrors a second delete that is actively polling
54-
# while a stale, permanently-failed pollable from a previous attempt lingers.
55-
def add_live_pollable(binding_type, binding, state: PollableJobModel::POLLING_STATE)
56-
operation = binding_type == :credential ? 'service_bindings.delete' : 'service_keys.delete'
57-
resource_type = binding_type == :credential ? 'service_bindings' : 'service_keys'
58-
delete_job = V3::DeleteBindingJob.new(binding_type, binding.guid, user_audit_info: user_audit_info)
59-
pjob = Jobs::Enqueuer.new(queue: Jobs::Queues.generic).enqueue_pollable(delete_job)
60-
pjob.update(state: state, operation: operation, resource_type: resource_type)
61-
pjob
62-
end
63-
6452
it { is_expected.to be_a_valid_job }
6553

6654
%i[credential key].each do |binding_type|
@@ -134,34 +122,6 @@ def add_live_pollable(binding_type, binding, state: PollableJobModel::POLLING_ST
134122
it_behaves_like 'does not retry the operation'
135123
end
136124

137-
context 'when a live pollable job is still driving the same operation' do
138-
# A previous delete attempt left a stale, permanently-failed pollable behind; a
139-
# second delete on the same binding is now actively polling. The stale row must
140-
# not trigger a spurious re-enqueue.
141-
it 'does not retry and leaves both pollables untouched' do
142-
scenario = prepare_stuck_binding(binding_type: binding_type)
143-
live_pjob = add_live_pollable(binding_type, scenario[:binding])
144-
145-
job.perform
146-
147-
expect(scenario[:binding].last_operation.reload.state).to eq('in progress')
148-
expect(scenario[:pjob].reload.state).to eq(PollableJobModel::FAILED_STATE)
149-
expect(live_pjob.reload.state).to eq(PollableJobModel::POLLING_STATE)
150-
expect(enqueuer).not_to have_received(:enqueue_pollable)
151-
end
152-
153-
it 'still retries once the live pollable is gone' do
154-
scenario = prepare_stuck_binding(binding_type: binding_type)
155-
live_pjob = add_live_pollable(binding_type, scenario[:binding])
156-
live_pjob.destroy
157-
158-
job.perform
159-
160-
expect(scenario[:pjob].reload.state).to eq(PollableJobModel::POLLING_STATE)
161-
expect(enqueuer).to have_received(:enqueue_pollable)
162-
end
163-
end
164-
165125
context 'when a binding delete job is stuck with state FAILED' do
166126
it 'resets the pollable job to POLLING and re-enqueues the original delete job' do
167127
scenario = prepare_stuck_binding(binding_type: binding_type)

‎spec/unit/jobs/runtime/service_operations_delete_stuck_in_progress_retry_spec.rb‎

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,6 @@ def prepare_stuck_service_instance(
4444
{ service_instance: service_instance, pjob: pjob, delayed_job: dj }
4545
end
4646

47-
# Attach an additional live pollable job (POLLING/PROCESSING, delayed_job NOT failed)
48-
# for the same instance + operation. Mirrors a second delete that is actively polling
49-
# while a stale, permanently-failed pollable from a previous attempt lingers.
50-
def add_live_pollable(service_instance, operation: 'service_instance.delete', state: PollableJobModel::POLLING_STATE)
51-
delete_job = V3::DeleteServiceInstanceJob.new(service_instance.guid, user_audit_info)
52-
pjob = Jobs::Enqueuer.new(queue: Jobs::Queues.generic).enqueue_pollable(delete_job)
53-
pjob.update(state: state, operation: operation)
54-
pjob
55-
end
56-
5747
shared_examples 'does not retry the operation' do
5848
it 'leaves the operation in progress, the pollable job untouched, and does not re-enqueue' do
5949
scenario = subject_scenario
@@ -120,34 +110,6 @@ def add_live_pollable(service_instance, operation: 'service_instance.delete', st
120110
it_behaves_like 'does not retry the operation'
121111
end
122112

123-
context 'when a live pollable job is still driving the same operation' do
124-
# A previous delete attempt left a stale, permanently-failed pollable behind; a
125-
# second delete on the same instance is now actively polling. The stale row must
126-
# not trigger a spurious re-enqueue.
127-
it 'does not retry and leaves both pollables untouched' do
128-
scenario = prepare_stuck_service_instance
129-
live_pjob = add_live_pollable(scenario[:service_instance])
130-
131-
job.perform
132-
133-
expect(scenario[:service_instance].last_operation.reload.state).to eq('in progress')
134-
expect(scenario[:pjob].reload.state).to eq(PollableJobModel::FAILED_STATE)
135-
expect(live_pjob.reload.state).to eq(PollableJobModel::POLLING_STATE)
136-
expect(enqueuer).not_to have_received(:enqueue_pollable)
137-
end
138-
139-
it 'still retries once the live pollable is gone' do
140-
scenario = prepare_stuck_service_instance
141-
live_pjob = add_live_pollable(scenario[:service_instance])
142-
live_pjob.destroy
143-
144-
job.perform
145-
146-
expect(scenario[:pjob].reload.state).to eq(PollableJobModel::POLLING_STATE)
147-
expect(enqueuer).to have_received(:enqueue_pollable)
148-
end
149-
end
150-
151113
context 'when a service instance delete job is stuck with state FAILED' do
152114
it 'resets the pollable job to POLLING and re-enqueues the original delete job' do
153115
scenario = prepare_stuck_service_instance

0 commit comments

Comments
 (0)