Skip to content

Commit abe8c6b

Browse files
committed
Skip stuck-operation recovery when a live pollable still drives it
The stuck-operation clock jobs (create cleanup, update fail, delete retry, binding-delete retry) matched a resource's pollable by resource_guid alone, so a permanently-failed pollable left by a previous operation wrongly caused a subsequent healthy operation to be failed or re-enqueued. Add a correlated NOT EXISTS guard that skips a resource when it still has a pollable actively driving the same operation (POLLING/PROCESSING backed by a non-failed delayed_job).
1 parent a6af7aa commit abe8c6b

8 files changed

Lines changed: 270 additions & 0 deletions

app/jobs/runtime/service_operations_binding_delete_stuck_in_progress_retry.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ 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)).
4041
select(
4142
Sequel[:jobs][:guid].as(:pollable_guid),
4243
Sequel[operation_table][:id].as(:op_id),
@@ -96,6 +97,25 @@ def default_maximum_duration_seconds
9697
Config.config.get(:broker_client_max_async_poll_duration_minutes).minutes
9798
end
9899

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+
99119
def logger
100120
@logger ||= Steno.logger('cc.background.service-operations-binding-delete-stuck-in-progress-retry')
101121
end

app/jobs/runtime/service_operations_create_in_progress_cleanup.rb

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ def cleanup_operations(operation_model, instance_model, foreign_key, jobs_operat
4040
# service instance that happen to share the same resource_guid
4141
# - delayed_jobs.failed_at IS NOT NULL: the delayed job permanently failed (exhausted max_attempts);
4242
# jobs still alive or locked have failed_at=NULL and must not be touched
43+
# - service_instances.guid NOT IN (live pollables for this operation): skip resources that still
44+
# have a POLLING/PROCESSING pollable driving this operation. A prior operation on the same
45+
# resource can leave a stale, permanently-failed pollable behind; without this guard that dead
46+
# row would match by resource_guid and cause the current healthy operation to be marked failed
4347
operation_table = operation_model.table_name
4448
instance_table = instance_model.table_name
4549

@@ -53,6 +57,7 @@ def cleanup_operations(operation_model, instance_model, foreign_key, jobs_operat
5357
where(Sequel[:jobs][:state] => [PollableJobModel::POLLING_STATE, PollableJobModel::FAILED_STATE]).
5458
where(Sequel[:jobs][:operation] => jobs_operation).
5559
exclude(Sequel[:delayed_jobs][:failed_at] => nil).
60+
exclude(live_pollable_exists(operation_model, instance_table, jobs_operation)).
5661
select(
5762
Sequel[:jobs][:guid].as(:pollable_guid),
5863
Sequel[operation_table][:id].as(:op_id),
@@ -104,6 +109,25 @@ def default_maximum_duration_seconds
104109
Config.config.get(:broker_client_max_async_poll_duration_minutes).minutes
105110
end
106111

112+
# NOT EXISTS guard: skip a resource if it still has a pollable job actively driving
113+
# THIS operation — state POLLING or PROCESSING AND backed by a delayed_job that has
114+
# NOT permanently failed (failed_at IS NULL, or no delayed_job row yet). A stale,
115+
# permanently-failed pollable left behind by a previous operation on the same
116+
# resource must NOT cause the current healthy operation to be marked failed. A
117+
# POLLING pollable whose delayed_job IS failed is itself stuck (the DB flip happened
118+
# before the failure hook could write FAILED) and must NOT count as live. Correlated
119+
# (resource_guid = instance.guid) so a NULL jobs.resource_guid elsewhere cannot
120+
# poison the result the way a NOT IN subquery would.
121+
def live_pollable_exists(operation_model, instance_table, jobs_operation)
122+
operation_model.db[:jobs].
123+
left_join(:delayed_jobs, guid: Sequel[:jobs][:delayed_job_guid]).
124+
where(Sequel[:jobs][:operation] => jobs_operation).
125+
where(Sequel[:jobs][:state] => [PollableJobModel::POLLING_STATE, PollableJobModel::PROCESSING_STATE]).
126+
where(Sequel[:delayed_jobs][:failed_at] => nil).
127+
where(Sequel[:jobs][:resource_guid] => Sequel[instance_table][:guid]).
128+
exists
129+
end
130+
107131
def logger
108132
@logger ||= Steno.logger('cc.background.service-operations-create-in-progress-cleanup')
109133
end

app/jobs/runtime/service_operations_delete_stuck_in_progress_retry.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ 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)).
3940
select(
4041
Sequel[:jobs][:guid].as(:pollable_guid),
4142
Sequel[operation_table][:id].as(:op_id),
@@ -95,6 +96,25 @@ def default_maximum_duration_seconds
9596
Config.config.get(:broker_client_max_async_poll_duration_minutes).minutes
9697
end
9798

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+
98118
def logger
99119
@logger ||= Steno.logger('cc.background.service-operations-delete-stuck-in-progress-retry')
100120
end

app/jobs/runtime/service_operations_update_stuck_in_progress_failed.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ def mark_stuck_in_progress_failed(operation_model, instance_model, foreign_key,
2929
where(Sequel[:jobs][:state] => [PollableJobModel::POLLING_STATE, PollableJobModel::FAILED_STATE]).
3030
where(Sequel[:jobs][:operation] => jobs_operation).
3131
exclude(Sequel[:delayed_jobs][:failed_at] => nil).
32+
exclude(live_pollable_exists(operation_model, instance_table, jobs_operation)).
3233
select(
3334
Sequel[:jobs][:guid].as(:pollable_guid),
3435
Sequel[operation_table][:id].as(:op_id),
@@ -71,6 +72,25 @@ def default_maximum_duration_seconds
7172
Config.config.get(:broker_client_max_async_poll_duration_minutes).minutes
7273
end
7374

75+
# NOT EXISTS guard: skip a resource if it still has a pollable job actively driving
76+
# THIS operation — state POLLING or PROCESSING AND backed by a delayed_job that has
77+
# NOT permanently failed (failed_at IS NULL, or no delayed_job row yet). A stale,
78+
# permanently-failed pollable left behind by a previous operation on the same
79+
# resource must NOT cause the current healthy operation to be marked failed. A
80+
# POLLING pollable whose delayed_job IS failed is itself stuck (the DB flip happened
81+
# before the failure hook could write FAILED) and must NOT count as live.
82+
# Correlated (resource_guid = instance.guid) so a NULL jobs.resource_guid elsewhere
83+
# cannot poison the result the way a NOT IN subquery would.
84+
def live_pollable_exists(operation_model, instance_table, jobs_operation)
85+
operation_model.db[:jobs].
86+
left_join(:delayed_jobs, guid: Sequel[:jobs][:delayed_job_guid]).
87+
where(Sequel[:jobs][:operation] => jobs_operation).
88+
where(Sequel[:jobs][:state] => [PollableJobModel::POLLING_STATE, PollableJobModel::PROCESSING_STATE]).
89+
where(Sequel[:delayed_jobs][:failed_at] => nil).
90+
where(Sequel[:jobs][:resource_guid] => Sequel[instance_table][:guid]).
91+
exists
92+
end
93+
7494
def logger
7595
@logger ||= Steno.logger('cc.background.service-operations-update-stuck-in-progress-failed')
7696
end

spec/unit/jobs/runtime/service_operations_binding_delete_stuck_in_progress_retry_spec.rb

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,18 @@ 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+
5264
it { is_expected.to be_a_valid_job }
5365

5466
%i[credential key].each do |binding_type|
@@ -122,6 +134,34 @@ def prepare_stuck_binding(
122134
it_behaves_like 'does not retry the operation'
123135
end
124136

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+
125165
context 'when a binding delete job is stuck with state FAILED' do
126166
it 'resets the pollable job to POLLING and re-enqueues the original delete job' do
127167
scenario = prepare_stuck_binding(binding_type: binding_type)

spec/unit/jobs/runtime/service_operations_create_in_progress_cleanup_spec.rb

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,24 @@ def prepare_stuck_service_instance(
5555
{ service_instance: service_instance, pjob: pjob, delayed_job: dj }
5656
end
5757

58+
# Attach an additional live pollable job (POLLING/PROCESSING, delayed_job NOT failed)
59+
# for the same instance + operation. Mirrors a second create actively polling while a
60+
# stale, permanently-failed pollable from a previous attempt lingers.
61+
def add_live_pollable(service_instance, operation: 'service_instance.create', state: PollableJobModel::POLLING_STATE)
62+
dj = Delayed::Job.create!(
63+
guid: SecureRandom.uuid,
64+
handler: 'fake',
65+
run_at: Time.now,
66+
queue: 'cc-generic'
67+
)
68+
create(:pollable_job_model,
69+
state: state,
70+
operation: operation,
71+
resource_guid: service_instance.guid,
72+
resource_type: 'service_instances',
73+
delayed_job_guid: dj.guid)
74+
end
75+
5876
shared_examples 'does not trigger orphan mitigation' do
5977
before { job.perform }
6078

@@ -119,6 +137,34 @@ def prepare_stuck_service_instance(
119137
it_behaves_like 'does not trigger orphan mitigation'
120138
end
121139

140+
context 'when a live pollable job is still driving the same operation' do
141+
# A previous create attempt left a stale, permanently-failed pollable behind; a
142+
# second create on the same instance is now actively polling. The stale row must
143+
# not cause the healthy current operation to be marked failed / mitigated.
144+
it 'does not mitigate and leaves both pollables untouched' do
145+
scenario = prepare_stuck_service_instance
146+
live_pjob = add_live_pollable(scenario[:service_instance])
147+
148+
job.perform
149+
150+
expect(scenario[:service_instance].last_operation.reload.state).to eq('in progress')
151+
expect(scenario[:pjob].reload.state).to eq(PollableJobModel::FAILED_STATE)
152+
expect(live_pjob.reload.state).to eq(PollableJobModel::POLLING_STATE)
153+
expect(fake_mitigator).not_to have_received(:cleanup_failed_provision)
154+
end
155+
156+
it 'still mitigates once the live pollable is gone' do
157+
scenario = prepare_stuck_service_instance
158+
live_pjob = add_live_pollable(scenario[:service_instance])
159+
live_pjob.destroy
160+
161+
job.perform
162+
163+
expect(scenario[:service_instance].last_operation.reload.state).to eq('failed')
164+
expect(fake_mitigator).to have_received(:cleanup_failed_provision).with(scenario[:service_instance])
165+
end
166+
end
167+
122168
context 'when a service instance create job is stuck with state FAILED' do
123169
it 'sets operation to failed, pollable job to FAILED, and triggers orphan mitigation' do
124170
scenario = prepare_stuck_service_instance

spec/unit/jobs/runtime/service_operations_delete_stuck_in_progress_retry_spec.rb

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,16 @@ 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+
4757
shared_examples 'does not retry the operation' do
4858
it 'leaves the operation in progress, the pollable job untouched, and does not re-enqueue' do
4959
scenario = subject_scenario
@@ -110,6 +120,34 @@ def prepare_stuck_service_instance(
110120
it_behaves_like 'does not retry the operation'
111121
end
112122

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+
113151
context 'when a service instance delete job is stuck with state FAILED' do
114152
it 'resets the pollable job to POLLING and re-enqueues the original delete job' do
115153
scenario = prepare_stuck_service_instance

0 commit comments

Comments
 (0)