Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: reduce contention when updating the contract_data_updated_at field for integrations #671

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

bethesque
Copy link
Member

@bethesque bethesque commented Mar 21, 2024

Reduce contention updating the contract_data_updated_at for integrations, which is causing some responses to be slow while locks are waited for.

This is tricky to test in a unit test, especially as the database we use for our unit tests (Sqlite) doesn't support SKIP LOCKED, so I manually tested it using the following code.

script/docker/db-rm.sh # remove any existing db
script/docker/db-start.sh
script/dev/console.rb postgres://postgres:postgres@localhost/postgres

td = PactBroker::Test::TestDataBuilder.new
td.create_consumer("A").create_provider("B").create_integration
td.create_consumer("C").create_provider("D").create_integration
td.create_consumer("E").create_provider("F").create_integration

things1 = [
	OpenStruct.new(consumer: OpenStruct.new(id: 1), provider: OpenStruct.new(id: 2)),
	OpenStruct.new(consumer: OpenStruct.new(id: 3), provider: OpenStruct.new(id: 4))
]

connection.transaction do
       # update the first and second integrations
	PactBroker::Integrations::Repository.new.set_contract_data_updated_at_for_multiple_integrations(things1)
	sleep 15
end

PactBroker::Integrations::Integration.order(:id).all

In another shell, within 15 seconds:

script/dev/console.rb postgres://postgres:postgres@localhost/postgres

# attempt to update second and third integrations
# only the third integration will be updated, as the second one is locked by the first transaction
things2 = [
	OpenStruct.new(consumer: OpenStruct.new(id: 3), provider: OpenStruct.new(id: 4)),
	OpenStruct.new(consumer: OpenStruct.new(id: 5), provider: OpenStruct.new(id: 6))
]

connection.transaction do
	PactBroker::Integrations::Repository.new.set_contract_data_updated_at_for_multiple_integrations(things2)
end

The second script will complete immediately, without waiting for the first one to sleep its 15 seconds. The first script will update integrations 1 and 2, while the second script will attempt to update 2 and 3, but will only actually update 3.

@bethesque bethesque requested review from vwong, tuan-pham and Saup21 March 21, 2024 04:41
@bethesque
Copy link
Member Author

Note that we do run all our tests against Postgres (multiple versions) as well as Sqlite (and MySQL, but let's not talk about that) in CI.

Copy link
Contributor

@vwong vwong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd vote for a major release, and prune the hairy bits off!

@bethesque
Copy link
Member Author

Major release for what hairy bits? What is backwards incompatible?

@bethesque bethesque merged commit ff72d03 into master Apr 2, 2024
17 checks passed
@bethesque bethesque deleted the chore/use-skip-lock-for-contract-data-updated branch April 2, 2024 00:58
@vwong
Copy link
Contributor

vwong commented Apr 3, 2024

I thought you were hinting at dropping support for other databases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants