Skip to content

Commit

Permalink
Prevent 500 on parallel service instance creation (cloudfoundry#3899)
Browse files Browse the repository at this point in the history
We observed 500 responses on POST /v3/service_instances when requests
run in parallel and the db is small.
This occurs if the one POST bypasses the other one and inserts the
record into the db after the first POST has passed the validate function
but before the actual insert. This leads to a PG::UniqueViolation: ERROR:
duplicate key value violates unique constraint which is not catched and
thus results in a 500 response. To prevent this, we add a
Sequel::UniqueConstraintViolation rescue to catch the PG::UniqueViolation error and to return an adequate 422 error "The service instance name is taken.."
  • Loading branch information
kathap authored Aug 2, 2024
1 parent 61ca706 commit 3a20cfa
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 0 deletions.
9 changes: 9 additions & 0 deletions app/models/services/service_instance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,15 @@ def name_clashes
end
end

def around_save
yield
rescue Sequel::UniqueConstraintViolation => e
raise e unless e.message.include?('si_space_id_name_index')

errors.add(:name, :unique)
raise validation_failed_error
end

def validate
validates_presence :name
validates_presence :space
Expand Down
17 changes: 17 additions & 0 deletions spec/unit/actions/v3/service_instance_create_managed_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,23 @@ module V3
end
end

context 'parallel creation of managed service instances' do
it 'ensures one creation is successful and the other fails due to name conflict' do
# First request, should succeed
expect do
action.precursor(message:, service_plan:)
end.not_to raise_error

# Mock the validation for the second request to simulate the race condition and trigger a unique constraint violation
allow_any_instance_of(ManagedServiceInstance).to receive(:validate).and_return(true)

# Second request, should fail with correct error message
expect do
action.precursor(message:, service_plan:)
end.to raise_error(ServiceInstanceCreateManaged::InvalidManagedServiceInstance, 'The service instance name is taken: si-test-name.')
end
end

context 'quotas' do
context 'when service instance limit has been reached for the space' do
before do
Expand Down

0 comments on commit 3a20cfa

Please sign in to comment.